Merge pull request #34 from cesarhernandezgt/tomcat-7.0.109-TT-patch-…

…cve-take-3

Ensure IOException on request read always triggers error handling

java/org/apache/catalina/connector/BadRequestException.java

@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.catalina.connector;
+
+import java.io.IOException;
+
+/**
+ * Extend IOException to identify it as being caused by a bad request from a remote client.
+ */
+public class BadRequestException extends IOException {
+
+    private static final long serialVersionUID = 1L;
+
+
+    // ------------------------------------------------------------ Constructors
+
+    /**
+     * Construct a new BadRequestException with no other information.
+     */
+    public BadRequestException() {
+        super();
+    }
+
+
+    /**
+     * Construct a new BadRequestException for the specified message.
+     *
+     * @param message Message describing this exception
+     */
+    public BadRequestException(String message) {
+        super(message);
+    }
+
+
+    /**
+     * Construct a new BadRequestException for the specified throwable.
+     *
+     * @param throwable Throwable that caused this exception
+     */
+    public BadRequestException(Throwable throwable) {
+        super(throwable);
+    }
+
+
+    /**
+     * Construct a new BadRequestException for the specified message and throwable.
+     *
+     * @param message   Message describing this exception
+     * @param throwable Throwable that caused this exception
+     */
+    public BadRequestException(String message, Throwable throwable) {
+        super(message, throwable);
+    }
+}

java/org/apache/catalina/connector/InputBuffer.java

@@ -23,6 +23,9 @@
 import java.security.PrivilegedExceptionAction;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
+import javax.servlet.RequestDispatcher;
 
 import org.apache.catalina.security.SecurityUtil;
 import org.apache.coyote.ActionCode;
@@ -287,10 +290,28 @@ public int realReadBytes(byte cbuf[], int off, int len)
             state = BYTE_STATE;
         }
 
-        int result = coyoteRequest.doRead(bb);
-
-        return result;
-
+        try {
+            return coyoteRequest.doRead(bb);
+        } catch (BadRequestException bre) {
+            // Set flag used by asynchronous processing to detect errors on non-container threads
+            coyoteRequest.setErrorException(bre);
+            // In synchronous processing, this exception may be swallowed by the application so set error flags here.
+            coyoteRequest.setAttribute(RequestDispatcher.ERROR_EXCEPTION, bre);
+            coyoteRequest.getResponse().setStatus(400);
+            coyoteRequest.getResponse().setError();
+            // Make the exception visible to the application
+            throw bre;
+        } catch (IOException ioe) {
+            // Set flag used by asynchronous processing to detect errors on non-container threads
+            coyoteRequest.setErrorException(ioe);
+            // In synchronous processing, this exception may be swallowed by the application so set error flags here.
+            coyoteRequest.setAttribute(RequestDispatcher.ERROR_EXCEPTION, ioe);
+            coyoteRequest.getResponse().setStatus(400);
+            coyoteRequest.getResponse().setError();
+            // Any other IOException on a read is almost always due to the remote client aborting the request.
+            // Make the exception visible to the application
+            throw new ClientAbortException(ioe);
+        }
     }
 
 

java/org/apache/catalina/core/StandardHostValve.java

@@ -425,11 +425,19 @@ protected void throwable(Request request, Response response,
                 }
             }
         } else {
-            // A custom error-page has not been defined for the exception
-            // that was thrown during request processing. Check if an
-            // error-page for error code 500 was specified and if so,
-            // send that page back as the response.
-            response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+
+            /*
+             *  A custom error-page has not been defined for the exception that was thrown during request processing.
+             *  Set the status to 500 if an error status has not already been set and check for custom error-page for
+             *  the status.
+             */
+            if (response.getStatus() < HttpServletResponse.SC_BAD_REQUEST) {
+                response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+            } else if (throwable instanceof org.apache.catalina.connector.BadRequestException) {
+                response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
+            }
+
+
             // The response is an error
             response.setError();
 

java/org/apache/coyote/Request.java

@@ -140,6 +140,13 @@ public Request() {
     private int available = 0;
 
     private RequestInfo reqProcessorMX=new RequestInfo(this);
+
+
+    /**
+     * Holds request body reading error exception.
+     */
+    private Exception errorException = null;
+
     // ------------------------------------------------------------- Properties
 
 
@@ -446,6 +453,33 @@ public int doRead(ByteChunk chunk)
         return n;
     }
 
+    // -------------------- Error tracking --------------------
+
+    /**
+     * Set the error Exception that occurred during the writing of the response processing.
+     *
+     * @param ex The exception that occurred
+     */
+    public void setErrorException(Exception ex) {
+        errorException = ex;
+    }
+
+
+    /**
+     * Get the Exception that occurred during the writing of the response.
+     *
+     * @return The exception that occurred
+     */
+    public Exception getErrorException() {
+        return errorException;
+    }
+
+
+    public boolean isExceptionPresent() {
+        return errorException != null;
+    }
+
+
 
     // -------------------- debug --------------------
 
@@ -528,6 +562,8 @@ public void recycle() {
         authType.recycle();
         attributes.clear();
 
+        errorException = null;
+
         startTime = -1;
     }
 

java/org/apache/coyote/http11/filters/ChunkedInputFilter.java

@@ -22,6 +22,7 @@
 import java.util.Locale;
 import java.util.Set;
 
+import org.apache.catalina.connector.BadRequestException;
 import org.apache.coyote.InputBuffer;
 import org.apache.coyote.Request;
 import org.apache.coyote.http11.Constants;
@@ -509,7 +510,9 @@ private boolean parseHeader() throws IOException {
                 colon = true;
             } else if (!HttpParser.isToken(chr)) {
                 // Non-token characters are illegal in header names
-                throw new IOException(sm.getString("chunkedInputFilter.invalidTrailerHeaderName"));
+                throwBadRequestException(sm.getString("chunkedInputFilter.invalidTrailerHeaderName"));
+            } else if (trailingHeaders.getEnd() >= trailingHeaders.getLimit()) {
+                throwBadRequestException(sm.getString("chunkedInputFilter.maxTrailer"));
             } else {
                 trailingHeaders.append(chr);
             }
@@ -573,6 +576,8 @@ private boolean parseHeader() throws IOException {
                     eol = true;
                 } else if (HttpParser.isControl(chr) && chr != Constants.HT) {
                     throw new IOException(sm.getString("chunkedInputFilter.invalidTrailerHeaderValue"));
+                } else if (trailingHeaders.getEnd() >= trailingHeaders.getLimit()) {
+                    throw new IOException(sm.getString("chunkedInputFilter.maxTrailer"));
                 } else if (chr == Constants.SP || chr == Constants.HT) {
                     trailingHeaders.append(chr);
                 } else {
@@ -598,6 +603,8 @@ private boolean parseHeader() throws IOException {
             chr = buf[pos];
             if ((chr != Constants.SP) && (chr != Constants.HT)) {
                 validLine = false;
+            } else if (trailingHeaders.getEnd() >= trailingHeaders.getLimit()) {
+                throwBadRequestException(sm.getString("chunkedInputFilter.maxTrailer"));
             } else {
                 eol = false;
                 // Copying one extra space in the buffer (since there must
@@ -627,6 +634,11 @@ private void throwIOException(String msg) throws IOException {
         throw new IOException(msg);
     }
 
+    private void throwBadRequestException(String msg) throws IOException {
+        error = true;
+        throw new BadRequestException(msg);
+    }
+
 
     private void throwEOFException(String msg) throws IOException {
         error = true;