You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ma...@apache.org on 2014/02/06 11:14:24 UTC

svn commit: r1565143 - in /commons/proper/fileupload/trunk/src: changes/ main/java/org/apache/commons/fileupload/ test/java/org/apache/commons/fileupload/

Author: markt
Date: Thu Feb  6 10:14:24 2014
New Revision: 1565143

URL: http://svn.apache.org/r1565143
Log:
Fix CVE-2014-0050. Specially crafted input can trigger a DoS if the buffer used by the <code>MultipartStream</code> is not big enough. When constructing <code>MultipartStream</code> enforce the requirements for buffer size by throwing an <code>IllegalArgumentException</code> if the requested buffer size is too small. This prevents the DoS.

Modified:
    commons/proper/fileupload/trunk/src/changes/changes.xml
    commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/FileUploadBase.java
    commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/MultipartStream.java
    commons/proper/fileupload/trunk/src/test/java/org/apache/commons/fileupload/MultipartStreamTest.java

Modified: commons/proper/fileupload/trunk/src/changes/changes.xml
URL: http://svn.apache.org/viewvc/commons/proper/fileupload/trunk/src/changes/changes.xml?rev=1565143&r1=1565142&r2=1565143&view=diff
==============================================================================
--- commons/proper/fileupload/trunk/src/changes/changes.xml (original)
+++ commons/proper/fileupload/trunk/src/changes/changes.xml Thu Feb  6 10:14:24 2014
@@ -51,6 +51,13 @@ The <action> type attribute can be add,u
       <action dev="markt" type="fix">
         Correct example in usage documentation so it compiles.
       </action>
+      <action dev="markt" type="fix">
+        Fix CVE-2014-0050. Specially crafted input can trigger a DoS if the
+        buffer used by the <code>MultipartStream</code> is not big enough. When
+        constructing <code>MultipartStream</code> enforce the requirements for
+        buffer size by throwing an <code>IllegalArgumentException</code> if the
+        requested buffer size is too small. This prevents the DoS. 
+      </action>
     </release>
     
     <release version="1.3" description="maintenance release, JDK1.5 update" date="2013-03-27">

Modified: commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/FileUploadBase.java
URL: http://svn.apache.org/viewvc/commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/FileUploadBase.java?rev=1565143&r1=1565142&r2=1565143&view=diff
==============================================================================
--- commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/FileUploadBase.java (original)
+++ commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/FileUploadBase.java Thu Feb  6 10:14:24 2014
@@ -991,7 +991,12 @@ public abstract class FileUploadBase {
             }
 
             notifier = new MultipartStream.ProgressNotifier(listener, requestSize);
-            multi = new MultipartStream(input, boundary, notifier);
+            try {
+                multi = new MultipartStream(input, boundary, notifier);
+            } catch (IllegalArgumentException iae) {
+                throw new InvalidContentTypeException(
+                        format("The boundary specified in the %s header is too long", CONTENT_TYPE), iae);
+            }
             multi.setHeaderEncoding(charEncoding);
 
             skipPreamble = true;
@@ -1183,7 +1188,7 @@ public abstract class FileUploadBase {
          * detail message.
          */
         public InvalidContentTypeException() {
-            // Nothing to do.
+            super();
         }
 
         /**
@@ -1196,6 +1201,9 @@ public abstract class FileUploadBase {
             super(message);
         }
 
+        public InvalidContentTypeException(String msg, Throwable cause) {
+            super(msg, cause);
+        }
     }
 
     /**

Modified: commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/MultipartStream.java
URL: http://svn.apache.org/viewvc/commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/MultipartStream.java?rev=1565143&r1=1565142&r2=1565143&view=diff
==============================================================================
--- commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/MultipartStream.java (original)
+++ commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/MultipartStream.java Thu Feb  6 10:14:24 2014
@@ -268,10 +268,8 @@ public class MultipartStream {
     /**
      * Creates a new instance.
      *
-     * @deprecated 1.2.1 Use {@link #MultipartStream(InputStream, byte[],
-     * org.apache.commons.fileupload.MultipartStream.ProgressNotifier)},
-     * or {@link #MultipartStream(InputStream, byte[], int,
-     * org.apache.commons.fileupload.MultipartStream.ProgressNotifier)}
+     * @deprecated 1.2.1 Use {@link #MultipartStream(InputStream, byte[], int,
+     * ProgressNotifier)}
      */
     @Deprecated
     public MultipartStream() {
@@ -292,10 +290,8 @@ public class MultipartStream {
      *                 <code>encapsulations</code>.
      * @param bufSize  The size of the buffer to be used, in bytes.
      *
-     * @see #MultipartStream(InputStream, byte[],
-     *   MultipartStream.ProgressNotifier)
      * @deprecated 1.2.1 Use {@link #MultipartStream(InputStream, byte[], int,
-     *  org.apache.commons.fileupload.MultipartStream.ProgressNotifier)}.
+     * ProgressNotifier)}.
      */
     @Deprecated
     public MultipartStream(InputStream input, byte[] boundary, int bufSize) {
@@ -317,8 +313,7 @@ public class MultipartStream {
      * @param pNotifier The notifier, which is used for calling the
      *                  progress listener, if any.
      *
-     * @see #MultipartStream(InputStream, byte[],
-     *     MultipartStream.ProgressNotifier)
+     * @throws IllegalArgumentException If the buffer size is too small
      */
     public MultipartStream(InputStream input,
             byte[] boundary,
@@ -331,9 +326,14 @@ public class MultipartStream {
 
         // We prepend CR/LF to the boundary to chop trailing CR/LF from
         // body-data tokens.
-        this.boundary = new byte[boundary.length + BOUNDARY_PREFIX.length];
         this.boundaryLength = boundary.length + BOUNDARY_PREFIX.length;
+        if (bufSize < this.boundaryLength + 1) {
+            throw new IllegalArgumentException(
+                    "The buffer size specified for the MultipartStream is too small");
+        }
+        this.boundary = new byte[this.boundaryLength];
         this.keepRegion = this.boundary.length;
+
         System.arraycopy(BOUNDARY_PREFIX, 0, this.boundary, 0,
                 BOUNDARY_PREFIX.length);
         System.arraycopy(boundary, 0, this.boundary, BOUNDARY_PREFIX.length,
@@ -352,8 +352,7 @@ public class MultipartStream {
      * @param pNotifier An object for calling the progress listener, if any.
      *
      *
-     * @see #MultipartStream(InputStream, byte[], int,
-     *     MultipartStream.ProgressNotifier)
+     * @see #MultipartStream(InputStream, byte[], int, ProgressNotifier)
      */
     MultipartStream(InputStream input,
             byte[] boundary,
@@ -368,10 +367,8 @@ public class MultipartStream {
      * @param boundary The token used for dividing the stream into
      *                 <code>encapsulations</code>.
      *
-     * @deprecated 1.2.1 Use {@link #MultipartStream(InputStream, byte[],
-     *  MultipartStream.ProgressNotifier)}.
-     * @see #MultipartStream(InputStream, byte[], int,
-     *  MultipartStream.ProgressNotifier)
+     * @deprecated 1.2.1 Use {@link #MultipartStream(InputStream, byte[], int,
+     *  ProgressNotifier)}.
      */
     @Deprecated
     public MultipartStream(InputStream input,

Modified: commons/proper/fileupload/trunk/src/test/java/org/apache/commons/fileupload/MultipartStreamTest.java
URL: http://svn.apache.org/viewvc/commons/proper/fileupload/trunk/src/test/java/org/apache/commons/fileupload/MultipartStreamTest.java?rev=1565143&r1=1565142&r2=1565143&view=diff
==============================================================================
--- commons/proper/fileupload/trunk/src/test/java/org/apache/commons/fileupload/MultipartStreamTest.java (original)
+++ commons/proper/fileupload/trunk/src/test/java/org/apache/commons/fileupload/MultipartStreamTest.java Thu Feb  6 10:14:24 2014
@@ -38,7 +38,8 @@ public class MultipartStreamTest {
         final byte[] contents = strData.getBytes();
         InputStream input = new ByteArrayInputStream(contents);
         byte[] boundary = BOUNDARY_TEXT.getBytes();
-        int iBufSize = boundary.length;
+        int iBufSize =
+                boundary.length + MultipartStream.BOUNDARY_PREFIX.length + 1;
         MultipartStream ms = new MultipartStream(
                 input,
                 boundary,
@@ -47,6 +48,21 @@ public class MultipartStreamTest {
         assertNotNull(ms);
     }
 
+    @Test(expected=IllegalArgumentException.class)
+    public void testSmallBuffer() throws Exception {
+        final String strData = "foobar";
+        final byte[] contents = strData.getBytes();
+        InputStream input = new ByteArrayInputStream(contents);
+        byte[] boundary = BOUNDARY_TEXT.getBytes();
+        int iBufSize = 1;
+        @SuppressWarnings("unused")
+        MultipartStream ms = new MultipartStream(
+                input,
+                boundary,
+                iBufSize,
+                new MultipartStream.ProgressNotifier(null, contents.length));
+    }
+
     @Test
     public void testTwoParamConstructor() throws Exception {
         final String strData = "foobar";