CVE-2013-4286

@@ -25,6 +25,8 @@
 import java.security.cert.X509Certificate;
 import java.util.concurrent.atomic.AtomicBoolean;
 
+import javax.servlet.http.HttpServletResponse;
+
 import org.apache.coyote.AbstractProcessor;
 import org.apache.coyote.ActionCode;
 import org.apache.coyote.AsyncContextCallback;
@@ -691,6 +693,7 @@ protected void prepareRequest() {
         // Set this every time in case limit has been changed via JMX
         headers.setLimit(endpoint.getMaxHeaderCount());
 
+        boolean contentLengthSet = false;
         int hCount = requestHeaderMessage.getInt();
         for(int i = 0 ; i < hCount ; i++) {
             String hName = null;
@@ -725,8 +728,15 @@ protected void prepareRequest() {
 
             if (hId == Constants.SC_REQ_CONTENT_LENGTH ||
                     (hId == -1 && tmpMB.equalsIgnoreCase("Content-Length"))) {
-                // just read the content-length header, so set it
-                request.setContentLength(vMB.getLong());
+                long cl = vMB.getLong();
+                if (contentLengthSet) {
+                    response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
+                    error = true;
+                } else {
+                    contentLengthSet = true;
+                    // Set the content-length header for the request
+                    request.setContentLength(cl);
+                }
             } else if (hId == Constants.SC_REQ_CONTENT_TYPE ||
                     (hId == -1 && tmpMB.equalsIgnoreCase("Content-Type"))) {
                 // just read the content-type header, so set it

@@ -1296,10 +1296,20 @@ protected void prepareRequest() {
 
         // Parse content-length header
         long contentLength = request.getContentLengthLong();
-        if (contentLength >= 0 && !contentDelimitation) {
-            getInputBuffer().addActiveFilter
-                (inputFilters[Constants.IDENTITY_FILTER]);
-            contentDelimitation = true;
+        if (contentLength >= 0) {
+            if (contentDelimitation) {
+                // contentDelimitation being true at this point indicates that
+                // chunked encoding is being used but chunked encoding should
+                // not be used with a content length. RFC 2616, section 4.4,
+                // bullet 3 states Content-Length must be ignored in this case -
+                // so remove it.
+                headers.removeHeader("content-length");
+                request.setContentLength(-1);
+            } else {
+                getInputBuffer().addActiveFilter
+                        (inputFilters[Constants.IDENTITY_FILTER]);
+                contentDelimitation = true;
+            }
         }
 
         MessageBytes valueMB = headers.getValue("host");

@@ -98,9 +98,20 @@ public void testKeepAlive() throws Exception {
         ajpClient.disconnect();
     }
 
+    @Test
+    public void testPost() throws Exception {
+        doTestPost(false, HttpServletResponse.SC_OK);
+    }
+
 
     @Test
-    public void testSimplePost() throws Exception {
+    public void testPostMultipleContentLength() throws Exception {
+        // Multiple content lengths
+        doTestPost(true, HttpServletResponse.SC_BAD_REQUEST);
+    }
+
+
+    public void doTestPost(boolean multipleCL, int expectedStatus) throws Exception {
 
         Tomcat tomcat = getTomcatInstance();
 
@@ -119,6 +130,9 @@ public void testSimplePost() throws Exception {
         TesterAjpMessage forwardMessage =
                 ajpClient.createForwardMessage("/echo-params.jsp", 4);
         forwardMessage.addHeader(0xA008, "9");
+        if (multipleCL) {
+            forwardMessage.addHeader(0xA008, "99");
+        }
         forwardMessage.addHeader(0xA007, "application/x-www-form-urlencoded");
         forwardMessage.end();
 
@@ -128,15 +142,20 @@ public void testSimplePost() throws Exception {
         TesterAjpMessage responseHeaders =
                 ajpClient.sendMessage(forwardMessage, bodyMessage);
 
-        // Expect 3 messages: headers, body, end
-        validateResponseHeaders(responseHeaders, 200);
-        // Skip the body
-        TesterAjpMessage responseBody = ajpClient.readMessage();
-        validateResponseBody(responseBody, "test - data");
-        validateResponseEnd(ajpClient.readMessage(), true);
+        validateResponseHeaders(responseHeaders, expectedStatus);
+        if (expectedStatus == HttpServletResponse.SC_OK) {
+            // Expect 3 messages: headers, body, end for a valid request
+            TesterAjpMessage responseBody = ajpClient.readMessage();
+            validateResponseBody(responseBody, "test - data");
+            validateResponseEnd(ajpClient.readMessage(), true);
+
+            // Double check the connection is still open
+            validateCpong(ajpClient.cping());
+        } else {
+            // Expect 2 messages: headers, end for an invalid request
+            validateResponseEnd(ajpClient.readMessage(), false);
+        }
 
-        // Double check the connection is still open
-        validateCpong(ajpClient.cping());
 
         ajpClient.disconnect();
     }

@@ -100,6 +100,54 @@ public void testWithTEBuffered() throws Exception {
     }
 
 
+    @Test
+    public void testWithTEChunked() throws Exception {
+        doTestWithTEChunked(false);
+    }
+
+
+    @Test
+    public void testWithTEChunkedWithCL() throws Exception {
+        // Should be ignored
+        doTestWithTEChunked(true);
+    }
+
+
+    private void doTestWithTEChunked(boolean withCL)
+            throws Exception {
+
+        Tomcat tomcat = getTomcatInstance();
+
+        // Use the normal Tomcat ROOT context
+        File root = new File("test/webapp-3.0");
+        tomcat.addWebapp("", root.getAbsolutePath());
+
+        tomcat.start();
+
+        String request =
+            "POST /echo-params.jsp HTTP/1.1" + SimpleHttpClient.CRLF +
+            "Host: any" + SimpleHttpClient.CRLF +
+            (withCL ? "Content-length: 1" + SimpleHttpClient.CRLF : "") +
+            "Transfer-encoding: chunked" + SimpleHttpClient.CRLF +
+            "Content-Type: application/x-www-form-urlencoded" +
+                    SimpleHttpClient.CRLF +
+            "Connection: close" + SimpleHttpClient.CRLF +
+            SimpleHttpClient.CRLF +
+            "9" + SimpleHttpClient.CRLF +
+            "test=data" + SimpleHttpClient.CRLF +
+            "0" + SimpleHttpClient.CRLF +
+            SimpleHttpClient.CRLF;
+
+        Client client = new Client(tomcat.getConnector().getLocalPort());
+        client.setRequest(new String[] {request});
+
+        client.connect();
+        client.processRequest();
+        assertTrue(client.isResponse200());
+        assertTrue(client.getResponseBody().contains("test - data"));
+    }
+
+
     @Test
     public void testWithTEIdentity() throws Exception {
         Tomcat tomcat = getTomcatInstance();

@@ -29,6 +29,7 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 
 import javax.servlet.RequestDispatcher;
+import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpUpgradeHandler;
 
 import org.apache.coyote.AbstractProcessor;
@@ -1110,6 +1111,7 @@ protected void prepareRequest() {
         // Set this every time in case limit has been changed via JMX
         headers.setLimit(endpoint.getMaxHeaderCount());
 
+        boolean contentLengthSet = false;
         int hCount = requestHeaderMessage.getInt();
         for(int i = 0 ; i < hCount ; i++) {
             String hName = null;
@@ -1144,9 +1146,15 @@ protected void prepareRequest() {
 
             if (hId == Constants.SC_REQ_CONTENT_LENGTH ||
                     (hId == -1 && tmpMB.equalsIgnoreCase("Content-Length"))) {
-                // just read the content-length header, so set it
                 long cl = vMB.getLong();
-                request.setContentLength(cl);
+                if (contentLengthSet) {
+                    response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
+                    error = true;
+                } else {
+                    contentLengthSet = true;
+                    // Set the content-length header for the request
+                    request.setContentLength(cl);
+                }
                 if (cl != 0) {
                     bodyPresent = true;
                 }

@@ -1263,10 +1263,20 @@ protected void prepareRequest() {
 
         // Parse content-length header
         long contentLength = request.getContentLengthLong();
-        if (contentLength >= 0 && !contentDelimitation) {
-            getInputBuffer().addActiveFilter
-                (inputFilters[Constants.IDENTITY_FILTER]);
-            contentDelimitation = true;
+        if (contentLength >= 0) {
+            if (contentDelimitation) {
+                // contentDelimitation being true at this point indicates that
+                // chunked encoding is being used but chunked encoding should
+                // not be used with a content length. RFC 2616, section 4.4,
+                // bullet 3 states Content-Length must be ignored in this case -
+                // so remove it.
+                headers.removeHeader("content-length");
+                request.setContentLength(-1);
+            } else {
+                getInputBuffer().addActiveFilter
+                        (inputFilters[Constants.IDENTITY_FILTER]);
+                contentDelimitation = true;
+            }
         }
 
         MessageBytes valueMB = headers.getValue("host");

@@ -98,9 +98,20 @@ public void testKeepAlive() throws Exception {
         ajpClient.disconnect();
     }
 
+    @Test
+    public void testPost() throws Exception {
+        doTestPost(false, HttpServletResponse.SC_OK);
+    }
+
 
     @Test
-    public void testSimplePost() throws Exception {
+    public void testPostMultipleContentLength() throws Exception {
+        // Multiple content lengths
+        doTestPost(true, HttpServletResponse.SC_BAD_REQUEST);
+    }
+
+
+    public void doTestPost(boolean multipleCL, int expectedStatus) throws Exception {
 
         Tomcat tomcat = getTomcatInstance();
 
@@ -119,6 +130,9 @@ public void testSimplePost() throws Exception {
         TesterAjpMessage forwardMessage =
                 ajpClient.createForwardMessage("/echo-params.jsp", 4);
         forwardMessage.addHeader(0xA008, "9");
+        if (multipleCL) {
+            forwardMessage.addHeader(0xA008, "99");
+        }
         forwardMessage.addHeader(0xA007, "application/x-www-form-urlencoded");
         forwardMessage.end();
 
@@ -128,15 +142,20 @@ public void testSimplePost() throws Exception {
         TesterAjpMessage responseHeaders =
                 ajpClient.sendMessage(forwardMessage, bodyMessage);
 
-        // Expect 3 messages: headers, body, end
-        validateResponseHeaders(responseHeaders, 200);
-        // Skip the body
-        TesterAjpMessage responseBody = ajpClient.readMessage();
-        validateResponseBody(responseBody, "test - data");
-        validateResponseEnd(ajpClient.readMessage(), true);
+        validateResponseHeaders(responseHeaders, expectedStatus);
+        if (expectedStatus == HttpServletResponse.SC_OK) {
+            // Expect 3 messages: headers, body, end for a valid request
+            TesterAjpMessage responseBody = ajpClient.readMessage();
+            validateResponseBody(responseBody, "test - data");
+            validateResponseEnd(ajpClient.readMessage(), true);
+
+            // Double check the connection is still open
+            validateCpong(ajpClient.cping());
+        } else {
+            // Expect 2 messages: headers, end for an invalid request
+            validateResponseEnd(ajpClient.readMessage(), false);
+        }
 
-        // Double check the connection is still open
-        validateCpong(ajpClient.cping());
 
         ajpClient.disconnect();
     }

@@ -103,6 +103,20 @@ public void testWithTEBuffered() throws Exception {
 
     @Test
     public void testWithTEChunked() throws Exception {
+        doTestWithTEChunked(false);
+    }
+
+
+    @Test
+    public void testWithTEChunkedWithCL() throws Exception {
+        // Should be ignored
+        doTestWithTEChunked(true);
+    }
+
+
+    private void doTestWithTEChunked(boolean withCL)
+            throws Exception {
+
         Tomcat tomcat = getTomcatInstance();
 
         // Use the normal Tomcat ROOT context
@@ -114,6 +128,7 @@ public void testWithTEChunked() throws Exception {
         String request =
             "POST /echo-params.jsp HTTP/1.1" + SimpleHttpClient.CRLF +
             "Host: any" + SimpleHttpClient.CRLF +
+            (withCL ? "Content-length: 1" + SimpleHttpClient.CRLF : "") +
             "Transfer-encoding: chunked" + SimpleHttpClient.CRLF +
             "Content-Type: application/x-www-form-urlencoded" +
                     SimpleHttpClient.CRLF +

@@ -29,6 +29,7 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 
 import javax.servlet.RequestDispatcher;
+import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpUpgradeHandler;
 
 import org.apache.coyote.AbstractProcessor;
@@ -1110,6 +1111,7 @@ protected void prepareRequest() {
         // Set this every time in case limit has been changed via JMX
         headers.setLimit(endpoint.getMaxHeaderCount());
 
+        boolean contentLengthSet = false;
         int hCount = requestHeaderMessage.getInt();
         for(int i = 0 ; i < hCount ; i++) {
             String hName = null;
@@ -1144,9 +1146,15 @@ protected void prepareRequest() {
 
             if (hId == Constants.SC_REQ_CONTENT_LENGTH ||
                     (hId == -1 && tmpMB.equalsIgnoreCase("Content-Length"))) {
-                // just read the content-length header, so set it
                 long cl = vMB.getLong();
-                request.setContentLength(cl);
+                if (contentLengthSet) {
+                    response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
+                    error = true;
+                } else {
+                    contentLengthSet = true;
+                    // Set the content-length header for the request
+                    request.setContentLength(cl);
+                }
                 if (cl != 0) {
                     bodyPresent = true;
                 }

@@ -1263,10 +1263,20 @@ protected void prepareRequest() {
 
         // Parse content-length header
         long contentLength = request.getContentLengthLong();
-        if (contentLength >= 0 && !contentDelimitation) {
-            getInputBuffer().addActiveFilter
-                (inputFilters[Constants.IDENTITY_FILTER]);
-            contentDelimitation = true;
+        if (contentLength >= 0) {
+            if (contentDelimitation) {
+                // contentDelimitation being true at this point indicates that
+                // chunked encoding is being used but chunked encoding should
+                // not be used with a content length. RFC 2616, section 4.4,
+                // bullet 3 states Content-Length must be ignored in this case -
+                // so remove it.
+                headers.removeHeader("content-length");
+                request.setContentLength(-1);
+            } else {
+                getInputBuffer().addActiveFilter
+                        (inputFilters[Constants.IDENTITY_FILTER]);
+                contentDelimitation = true;
+            }
         }
 
         MessageBytes valueMB = headers.getValue("host");

@@ -98,9 +98,20 @@ public void testKeepAlive() throws Exception {
         ajpClient.disconnect();
     }
 
+    @Test
+    public void testPost() throws Exception {
+        doTestPost(false, HttpServletResponse.SC_OK);
+    }
+
 
     @Test
-    public void testSimplePost() throws Exception {
+    public void testPostMultipleContentLength() throws Exception {
+        // Multiple content lengths
+        doTestPost(true, HttpServletResponse.SC_BAD_REQUEST);
+    }
+
+
+    public void doTestPost(boolean multipleCL, int expectedStatus) throws Exception {
 
         Tomcat tomcat = getTomcatInstance();
 
@@ -119,6 +130,9 @@ public void testSimplePost() throws Exception {
         TesterAjpMessage forwardMessage =
                 ajpClient.createForwardMessage("/echo-params.jsp", 4);
         forwardMessage.addHeader(0xA008, "9");
+        if (multipleCL) {
+            forwardMessage.addHeader(0xA008, "99");
+        }
         forwardMessage.addHeader(0xA007, "application/x-www-form-urlencoded");
         forwardMessage.end();
 
@@ -128,15 +142,20 @@ public void testSimplePost() throws Exception {
         TesterAjpMessage responseHeaders =
                 ajpClient.sendMessage(forwardMessage, bodyMessage);
 
-        // Expect 3 messages: headers, body, end
-        validateResponseHeaders(responseHeaders, 200);
-        // Skip the body
-        TesterAjpMessage responseBody = ajpClient.readMessage();
-        validateResponseBody(responseBody, "test - data");
-        validateResponseEnd(ajpClient.readMessage(), true);
+        validateResponseHeaders(responseHeaders, expectedStatus);
+        if (expectedStatus == HttpServletResponse.SC_OK) {
+            // Expect 3 messages: headers, body, end for a valid request
+            TesterAjpMessage responseBody = ajpClient.readMessage();
+            validateResponseBody(responseBody, "test - data");
+            validateResponseEnd(ajpClient.readMessage(), true);
+
+            // Double check the connection is still open
+            validateCpong(ajpClient.cping());
+        } else {
+            // Expect 2 messages: headers, end for an invalid request
+            validateResponseEnd(ajpClient.readMessage(), false);
+        }
 
-        // Double check the connection is still open
-        validateCpong(ajpClient.cping());
 
         ajpClient.disconnect();
     }

@@ -103,6 +103,20 @@ public void testWithTEBuffered() throws Exception {
 
     @Test
     public void testWithTEChunked() throws Exception {
+        doTestWithTEChunked(false);
+    }
+
+
+    @Test
+    public void testWithTEChunkedWithCL() throws Exception {
+        // Should be ignored
+        doTestWithTEChunked(true);
+    }
+
+
+    private void doTestWithTEChunked(boolean withCL)
+            throws Exception {
+
         Tomcat tomcat = getTomcatInstance();
 
         // Use the normal Tomcat ROOT context
@@ -114,6 +128,7 @@ public void testWithTEChunked() throws Exception {
         String request =
             "POST /echo-params.jsp HTTP/1.1" + SimpleHttpClient.CRLF +
             "Host: any" + SimpleHttpClient.CRLF +
+            (withCL ? "Content-length: 1" + SimpleHttpClient.CRLF : "") +
             "Transfer-encoding: chunked" + SimpleHttpClient.CRLF +
             "Content-Type: application/x-www-form-urlencoded" +
                     SimpleHttpClient.CRLF +

@@ -707,9 +707,7 @@ protected void prepareRequest() {
             if (hId == Constants.SC_REQ_CONTENT_LENGTH ||
                     (hId == -1 && tmpMB.equalsIgnoreCase("Content-Length"))) {
                 // just read the content-length header, so set it
-                long cl = vMB.getLong();
-                if(cl < Integer.MAX_VALUE)
-                    request.setContentLength( (int)cl );
+                request.setContentLength(vMB.getLong());
             } else if (hId == Constants.SC_REQ_CONTENT_TYPE ||
                     (hId == -1 && tmpMB.equalsIgnoreCase("Content-Type"))) {
                 // just read the content-type header, so set it

@@ -272,7 +272,7 @@ public void setCharacterEncoding(String enc) {
     }
 
 
-    public void setContentLength(int len) {
+    public void setContentLength(long len) {
         this.contentLength = len;
     }
 

@@ -176,6 +176,11 @@
         APR/native and HTTP APR/native connectors no longer support multiple
         poller threads. Both connectors now use a single poller thread. (markt)  
       </update>
+      <fix>
+        Internally, content length is managed as a <code>long</code>. Fix a few
+        places in the AJP connector where this was restricted to an
+        <code>int</code>. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper">

@@ -688,9 +688,7 @@ protected void prepareRequest() {
             if (hId == Constants.SC_REQ_CONTENT_LENGTH ||
                     (hId == -1 && tmpMB.equalsIgnoreCase("Content-Length"))) {
                 // just read the content-length header, so set it
-                long cl = vMB.getLong();
-                if(cl < Integer.MAX_VALUE)
-                    request.setContentLength( (int)cl );
+                request.setContentLength(vMB.getLong());
             } else if (hId == Constants.SC_REQ_CONTENT_TYPE ||
                     (hId == -1 && tmpMB.equalsIgnoreCase("Content-Type"))) {
                 // just read the content-type header, so set it

@@ -296,7 +296,7 @@ public void setCharacterEncoding(String enc) {
     }
 
 
-    public void setContentLength(int len) {
+    public void setContentLength(long len) {
         this.contentLength = len;
     }