Merge pull request #35 from cesarhernandezgt/master

backport fix for CVE-2023-46589

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

@@ -302,10 +302,38 @@ public int realReadBytes(byte cbuf[], int off, int len)
         if(state == INITIAL_STATE)
             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.
+            // CH: We can't use ERROR_EXCEPTION because Tomcat 6 implements Servlet 2.5 and ERROR_EXCEPTION was
+            // introduced in Servlet 3.0
+            // coyoteRequest.setAttribute(RequestDispatcher.ERROR_EXCEPTION, bre);
+
+            coyoteRequest.getResponse().setStatus(400);
+            coyoteRequest.getResponse().setErrorTrue();
+
+            // 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.
+            // CH: We can't use ERROR_EXCEPTION because Tomcat 6 implements Servlet 2.5 and ERROR_EXCEPTION was
+            // introduced in Servlet 3.0
+            // coyoteRequest.setAttribute(RequestDispatcher.ERROR_EXCEPTION, bre);
+
+            coyoteRequest.getResponse().setStatus(400);
+            coyoteRequest.getResponse().setErrorTrue();
+
+            // 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/connector/Response.java

@@ -214,10 +214,6 @@ public void setContext(Context context) {
      */
     private boolean isCharacterEncodingSet = false;
 
-    /**
-     * The error flag.
-     */
-    protected boolean error = false;
 
 
     /**
@@ -264,7 +260,6 @@ public void recycle() {
         usingWriter = false;
         appCommitted = false;
         included = false;
-        error = false;
         isCharacterEncodingSet = false;
 
         cookies.clear();
@@ -453,15 +448,15 @@ public boolean isClosed() {
      * Set the error flag.
      */
     public void setError() {
-        error = true;
+        getCoyoteResponse().setErrorTrue();
     }
 
 
     /**
      * Error flag accessor.
      */
     public boolean isError() {
-        return error;
+        return getCoyoteResponse().getErrorFlag();
     }
 
 
@@ -1326,6 +1321,16 @@ public void sendError(int status, String message)
      */
     public void sendRedirect(String location) 
         throws IOException {
+        sendRedirect(location, SC_FOUND);
+
+    }
+
+    /**
+     * Internal method that allows a redirect to be sent with a status other
+     * than {@link HttpServletResponse#SC_FOUND} (302). No attempt is made to
+     * validate the status code.
+     */
+    public void sendRedirect(String location, int status) throws IOException {
 
         if (isCommitted())
             throw new IllegalStateException
@@ -1341,7 +1346,7 @@ public void sendRedirect(String location)
         // Generate a temporary redirect to the specified location
         try {
             String absolute = toAbsolute(location);
-            setStatus(SC_FOUND);
+            setStatus(status);
             setHeader("Location", absolute);
         } catch (IllegalArgumentException e) {
             setStatus(SC_NOT_FOUND);

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

@@ -275,11 +275,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/catalina/valves/ErrorReportValve.java

@@ -121,8 +121,11 @@ public void invoke(Request request, Response response)
                 ;
             }
 
-            response.sendError
-                (HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+            if (throwable instanceof org.apache.catalina.connector.BadRequestException) {
+                response.sendError (HttpServletResponse.SC_BAD_REQUEST);
+            }else{
+                response.sendError (HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+            }
 
         }
 

java/org/apache/coyote/Request.java

@@ -145,6 +145,13 @@ public Request() {
     private int available = 0;
 
     private RequestInfo reqProcessorMX=new RequestInfo(this);
+
+
+    /**
+     * Holds request body reading error exception.
+     */
+    private Exception errorException = null;
+
     // ------------------------------------------------------------- Properties
 
 
@@ -431,6 +438,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 --------------------
 
@@ -510,6 +544,7 @@ public void recycle() {
         remoteUser.recycle();
         authType.recycle();
         attributes.clear();
+        errorException = null;
     }
 
     // -------------------- Info  --------------------

java/org/apache/coyote/Response.java

@@ -127,6 +127,11 @@ public Response() {
 
     protected Request req;
 
+    /**
+     * The error flag.
+     */
+    protected boolean error = false;
+
     // ------------------------------------------------------------- Properties
 
     public Request getRequest() {
@@ -255,6 +260,14 @@ public boolean isExceptionPresent() {
         return ( errorException != null );
     }
 
+    public void setErrorTrue() {
+        error = true;
+    }
+
+   public boolean getErrorFlag(){
+        return error;
+   }
+
 
     /**
      * Set request URI that caused an error during
@@ -551,6 +564,7 @@ public void recycle() {
         errorURI = null;
         headers.clear();
 
+        error = false;
         // update counters
         bytesWritten=0;
     }

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

@@ -20,6 +20,7 @@
 import java.io.IOException;
 import java.util.Locale;
 
+import org.apache.catalina.connector.BadRequestException;
 import org.apache.tomcat.util.buf.ByteChunk;
 import org.apache.tomcat.util.buf.HexUtils;
 import org.apache.coyote.InputBuffer;
@@ -477,7 +478,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);
             }
@@ -543,6 +546,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 {
@@ -568,6 +573,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
@@ -597,6 +604,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;