File tomcat-8.0-CVE-2023-46589.patch of Package tomcat.37363
Index: apache-tomcat-8.0.53-src/java/org/apache/catalina/connector/InputBuffer.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/catalina/connector/InputBuffer.java
+++ apache-tomcat-8.0.53-src/java/org/apache/catalina/connector/InputBuffer.java
@@ -25,6 +25,7 @@ import java.util.concurrent.ConcurrentHa
import java.util.concurrent.atomic.AtomicBoolean;
import javax.servlet.ReadListener;
+import javax.servlet.RequestDispatcher;
import org.apache.catalina.security.SecurityUtil;
import org.apache.coyote.ActionCode;
@@ -326,6 +327,7 @@ public class InputBuffer extends Reader
*
* @throws IOException An underlying IOException occurred
*/
+ @SuppressWarnings("deprecation")
@Override
public int realReadBytes(byte cbuf[], int off, int len)
throws IOException {
@@ -341,19 +343,35 @@ public class InputBuffer extends Reader
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);
+ }
}
public int readByte()
throws IOException {
- if (closed) {
- throw new IOException(sm.getString("inputBuffer.streamClosed"));
- }
+ throwIfClosed();
return bb.substract();
}
@@ -362,9 +380,7 @@ public class InputBuffer extends Reader
public int read(byte[] b, int off, int len)
throws IOException {
- if (closed) {
- throw new IOException(sm.getString("inputBuffer.streamClosed"));
- }
+ throwIfClosed();
return bb.substract(b, off, len);
}
@@ -439,9 +455,7 @@ public class InputBuffer extends Reader
public int read()
throws IOException {
- if (closed) {
- throw new IOException(sm.getString("inputBuffer.streamClosed"));
- }
+ throwIfClosed();
return cb.substract();
}
@@ -451,9 +465,7 @@ public class InputBuffer extends Reader
public int read(char[] cbuf)
throws IOException {
- if (closed) {
- throw new IOException(sm.getString("inputBuffer.streamClosed"));
- }
+ throwIfClosed();
return read(cbuf, 0, cbuf.length);
}
@@ -463,9 +475,7 @@ public class InputBuffer extends Reader
public int read(char[] cbuf, int off, int len)
throws IOException {
- if (closed) {
- throw new IOException(sm.getString("inputBuffer.streamClosed"));
- }
+ throwIfClosed();
return cb.substract(cbuf, off, len);
}
@@ -475,10 +485,7 @@ public class InputBuffer extends Reader
public long skip(long n)
throws IOException {
-
- if (closed) {
- throw new IOException(sm.getString("inputBuffer.streamClosed"));
- }
+ throwIfClosed();
if (n < 0) {
throw new IllegalArgumentException();
@@ -514,9 +521,8 @@ public class InputBuffer extends Reader
public boolean ready()
throws IOException {
- if (closed) {
- throw new IOException(sm.getString("inputBuffer.streamClosed"));
- }
+ throwIfClosed();
+
if (state == INITIAL_STATE) {
state = CHAR_STATE;
}
@@ -534,9 +540,7 @@ public class InputBuffer extends Reader
public void mark(int readAheadLimit)
throws IOException {
- if (closed) {
- throw new IOException(sm.getString("inputBuffer.streamClosed"));
- }
+ throwIfClosed();
if (cb.getLength() <= 0) {
cb.setOffset(0);
@@ -559,9 +563,7 @@ public class InputBuffer extends Reader
public void reset()
throws IOException {
- if (closed) {
- throw new IOException(sm.getString("inputBuffer.streamClosed"));
- }
+ throwIfClosed();
if (state == CHAR_STATE) {
if (markPos < 0) {
@@ -623,4 +625,11 @@ public class InputBuffer extends Reader
}
+ private void throwIfClosed() throws IOException {
+ if (closed) {
+ IOException ioe = new IOException(sm.getString("inputBuffer.streamClosed"));
+ coyoteRequest.setErrorException(ioe);
+ throw ioe;
+ }
+ }
}
Index: apache-tomcat-8.0.53-src/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
+++ apache-tomcat-8.0.53-src/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
@@ -423,6 +423,82 @@ public class TestChunkedInputFilter exte
}
}
+ @Test
+ public void testTrailerHeaderNameNotTokenThrowException() throws Exception {
+ doTestTrailerHeaderNameNotToken(false);
+ }
+
+ @Test
+ public void testTrailerHeaderNameNotTokenSwallowException() throws Exception {
+ doTestTrailerHeaderNameNotToken(true);
+ }
+
+ private void doTestTrailerHeaderNameNotToken(boolean swallowException) throws Exception {
+
+ // Setup Tomcat instance
+ Tomcat tomcat = getTomcatInstance();
+
+ // No file system docBase required
+ Context ctx = tomcat.addContext("", null);
+
+ Tomcat.addServlet(ctx, "servlet", new SwallowBodyServlet(swallowException));
+ ctx.addServletMappingDecoded("/", "servlet");
+
+ tomcat.start();
+
+ String[] request = new String[]{
+ "POST / HTTP/1.1" + SimpleHttpClient.CRLF +
+ "Host: localhost" + SimpleHttpClient.CRLF +
+ "Transfer-encoding: chunked" + SimpleHttpClient.CRLF +
+ "Content-Type: application/x-www-form-urlencoded" + SimpleHttpClient.CRLF +
+ "Connection: close" + SimpleHttpClient.CRLF +
+ SimpleHttpClient.CRLF +
+ "3" + SimpleHttpClient.CRLF +
+ "a=0" + SimpleHttpClient.CRLF +
+ "4" + SimpleHttpClient.CRLF +
+ "&b=1" + SimpleHttpClient.CRLF +
+ "0" + SimpleHttpClient.CRLF +
+ "x@trailer: Test" + SimpleHttpClient.CRLF +
+ SimpleHttpClient.CRLF };
+
+ TrailerClient client = new TrailerClient(tomcat.getConnector().getLocalPort());
+ client.setRequest(request);
+
+ client.connect();
+ client.processRequest();
+ // Expected to fail because of invalid trailer header name
+ Assert.assertTrue(client.getResponseLine(), client.isResponse400());
+ }
+
+ private static class SwallowBodyServlet extends HttpServlet {
+ private static final long serialVersionUID = 1L;
+
+ private final boolean swallowException;
+
+ SwallowBodyServlet(boolean swallowException) {
+ this.swallowException = swallowException;
+ }
+
+ @Override
+ protected void doPost(HttpServletRequest req, HttpServletResponse resp)
+ throws ServletException, IOException {
+ resp.setContentType("text/plain");
+ PrintWriter pw = resp.getWriter();
+
+ // Read the body
+ InputStream is = req.getInputStream();
+ try {
+ while (is.read() > -1) {
+ }
+ pw.write("OK");
+ } catch (IOException ioe) {
+ if (!swallowException) {
+ throw ioe;
+ }
+ }
+ }
+ }
+
private static class EchoHeaderServlet extends HttpServlet {
private static final long serialVersionUID = 1L;
Index: apache-tomcat-8.0.53-src/java/org/apache/catalina/connector/BadRequestException.java
===================================================================
--- /dev/null
+++ apache-tomcat-8.0.53-src/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);
+ }
+}
Index: apache-tomcat-8.0.53-src/java/org/apache/catalina/connector/ClientAbortException.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/catalina/connector/ClientAbortException.java
+++ apache-tomcat-8.0.53-src/java/org/apache/catalina/connector/ClientAbortException.java
@@ -16,15 +16,13 @@
*/
package org.apache.catalina.connector;
-import java.io.IOException;
-
/**
- * Wrap an IOException identifying it as being caused by an abort
+ * Wrap an BadRequestException identifying it as being caused by an abort
* of a request by a remote client.
*
* @author Glenn L. Nielsen
*/
-public final class ClientAbortException extends IOException {
+public final class ClientAbortException extends BadRequestException {
private static final long serialVersionUID = 1L;
Index: apache-tomcat-8.0.53-src/java/org/apache/catalina/core/ApplicationDispatcher.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/catalina/core/ApplicationDispatcher.java
+++ apache-tomcat-8.0.53-src/java/org/apache/catalina/core/ApplicationDispatcher.java
@@ -40,7 +40,7 @@ import org.apache.catalina.Context;
import org.apache.catalina.Globals;
import org.apache.catalina.InstanceEvent;
import org.apache.catalina.Wrapper;
-import org.apache.catalina.connector.ClientAbortException;
+import org.apache.catalina.connector.BadRequestException;
import org.apache.catalina.connector.Request;
import org.apache.catalina.connector.RequestFacade;
import org.apache.catalina.connector.Response;
@@ -718,7 +718,7 @@ final class ApplicationDispatcher implem
// Servlet Service Method is called by the FilterChain
support.fireInstanceEvent(InstanceEvent.AFTER_DISPATCH_EVENT,
servlet, request, response);
- } catch (ClientAbortException e) {
+ } catch (BadRequestException e) {
support.fireInstanceEvent(InstanceEvent.AFTER_DISPATCH_EVENT,
servlet, request, response);
ioException = e;
@@ -739,7 +739,7 @@ final class ApplicationDispatcher implem
support.fireInstanceEvent(InstanceEvent.AFTER_DISPATCH_EVENT,
servlet, request, response);
Throwable rootCause = StandardWrapper.getRootCause(e);
- if (!(rootCause instanceof ClientAbortException)) {
+ if (!(rootCause instanceof BadRequestException)) {
wrapper.getLogger().error(sm.getString("applicationDispatcher.serviceException",
wrapper.getName()), rootCause);
}
Index: apache-tomcat-8.0.53-src/java/org/apache/catalina/connector/CoyoteAdapter.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/catalina/connector/CoyoteAdapter.java
+++ apache-tomcat-8.0.53-src/java/org/apache/catalina/connector/CoyoteAdapter.java
@@ -322,6 +322,10 @@ public class CoyoteAdapter implements Ad
readListener != null) {
readListener.onAllDataRead();
}
+ // User code may have swallowed an IOException
+ if (response.getCoyoteResponse().isExceptionPresent()) {
+ throw response.getCoyoteResponse().getErrorException();
+ }
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
writeListener.onError(t);
@@ -344,6 +348,10 @@ public class CoyoteAdapter implements Ad
if (request.isFinished() && req.sendAllDataReadEvent()) {
readListener.onAllDataRead();
}
+ // User code may have swallowed an IOException
+ if (request.getCoyoteRequest().isExceptionPresent()) {
+ throw request.getCoyoteRequest().getErrorException();
+ }
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
readListener.onError(t);
Index: apache-tomcat-8.0.53-src/java/org/apache/catalina/connector/OutputBuffer.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/catalina/connector/OutputBuffer.java
+++ apache-tomcat-8.0.53-src/java/org/apache/catalina/connector/OutputBuffer.java
@@ -393,6 +393,7 @@ public class OutputBuffer extends Writer
// An IOException on a write is almost always due to
// the remote client aborting the request. Wrap this
// so that it can be handled better by the error dispatcher.
+ coyoteResponse.setErrorException(e);
throw new ClientAbortException(e);
}
}
Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/Request.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/Request.java
+++ apache-tomcat-8.0.53-src/java/org/apache/coyote/Request.java
@@ -142,6 +142,10 @@ public final class Request {
private final RequestInfo reqProcessorMX=new RequestInfo(this);
+ /**
+ * Holds request body reading error exception.
+ */
+ private Exception errorException = null;
volatile ReadListener listener;
@@ -480,6 +484,32 @@ public final class Request {
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 --------------------
Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/Response.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/Response.java
+++ apache-tomcat-8.0.53-src/java/org/apache/coyote/Response.java
@@ -21,6 +21,7 @@ import java.io.StringReader;
import java.nio.charset.Charset;
import java.util.Locale;
import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
import javax.servlet.WriteListener;
@@ -111,15 +112,46 @@ public final class Response {
private long commitTime = -1;
/**
- * Holds request error exception.
+ * Holds response writing error exception.
*/
- Exception errorException = null;
+ private Exception errorException = null;
/**
* Has the charset been explicitly set.
*/
boolean charsetSet = false;
+ /**
+ * With the introduction of async processing and the possibility of
+ * non-container threads calling sendError() tracking the current error
+ * state and ensuring that the correct error page is called becomes more
+ * complicated. This state attribute helps by tracking the current error
+ * state and informing callers that attempt to change state if the change
+ * was successful or if another thread got there first.
+ *
+ * <pre>
+ * The state machine is very simple:
+ *
+ * 0 - NONE
+ * 1 - NOT_REPORTED
+ * 2 - REPORTED
+ *
+ *
+ * -->---->-- >NONE
+ * | | |
+ * | | | setError()
+ * ^ ^ |
+ * | | \|/
+ * | |-<-NOT_REPORTED
+ * | |
+ * ^ | report()
+ * | |
+ * | \|/
+ * |----<----REPORTED
+ * </pre>
+ */
+ private final AtomicInteger errorState = new AtomicInteger(0);
+
Request req;
// ------------------------------------------------------------- Properties
@@ -256,7 +288,37 @@ public final class Response {
public boolean isExceptionPresent() {
- return ( errorException != null );
+ return errorException != null;
+ }
+
+
+ /**
+ * Set the error flag.
+ *
+ * @return <code>false</code> if the error flag was already set
+ */
+ public boolean setError() {
+ return errorState.compareAndSet(0, 1);
+ }
+
+
+ /**
+ * Error flag accessor.
+ *
+ * @return <code>true</code> if the response has encountered an error
+ */
+ public boolean isError() {
+ return errorState.get() > 0;
+ }
+
+
+ public boolean isErrorReportRequired() {
+ return errorState.get() == 1;
+ }
+
+
+ public boolean setErrorReported() {
+ return errorState.compareAndSet(1, 2);
}
@@ -507,6 +569,7 @@ public final class Response {
commited = false;
commitTime = -1;
errorException = null;
+ errorState.set(0);
headers.clear();
// Servlet 3.1 non-blocking write listener
listener = null;
Index: apache-tomcat-8.0.53-src/java/org/apache/catalina/connector/Response.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/catalina/connector/Response.java
+++ apache-tomcat-8.0.53-src/java/org/apache/catalina/connector/Response.java
@@ -34,7 +34,6 @@ import java.util.List;
import java.util.Locale;
import java.util.TimeZone;
import java.util.Vector;
-import java.util.concurrent.atomic.AtomicInteger;
import javax.servlet.ServletOutputStream;
import javax.servlet.SessionTrackingMode;
@@ -182,38 +181,6 @@ public class Response implements HttpSer
private boolean isCharacterEncodingSet = false;
/**
- * With the introduction of async processing and the possibility of
- * non-container threads calling sendError() tracking the current error
- * state and ensuring that the correct error page is called becomes more
- * complicated. This state attribute helps by tracking the current error
- * state and informing callers that attempt to change state if the change
- * was successful or if another thread got there first.
- *
- * <pre>
- * The state machine is very simple:
- *
- * 0 - NONE
- * 1 - NOT_REPORTED
- * 2 - REPORTED
- *
- *
- * -->---->-- >NONE
- * | | |
- * | | | setError()
- * ^ ^ |
- * | | \|/
- * | |-<-NOT_REPORTED
- * | |
- * ^ | report()
- * | |
- * | \|/
- * |----<----REPORTED
- * </pre>
- */
- private final AtomicInteger errorState = new AtomicInteger(0);
-
-
- /**
* Using output stream flag.
*/
protected boolean usingOutputStream = false;
@@ -251,7 +218,6 @@ public class Response implements HttpSer
usingWriter = false;
appCommitted = false;
included = false;
- errorState.set(0);
isCharacterEncodingSet = false;
if (Globals.IS_SECURITY_ENABLED || Connector.RECYCLE_FACADES) {
@@ -399,7 +365,7 @@ public class Response implements HttpSer
* Set the error flag.
*/
public boolean setError() {
- boolean result = errorState.compareAndSet(0, 1);
+ boolean result = getCoyoteResponse().setError();
if (result) {
Wrapper wrapper = getRequest().getWrapper();
if (wrapper != null) {
@@ -414,17 +380,17 @@ public class Response implements HttpSer
* Error flag accessor.
*/
public boolean isError() {
- return errorState.get() > 0;
+ return getCoyoteResponse().isError();
}
public boolean isErrorReportRequired() {
- return errorState.get() == 1;
+ return getCoyoteResponse().isErrorReportRequired();
}
public boolean setErrorReported() {
- return errorState.compareAndSet(1, 2);
+ return getCoyoteResponse().setErrorReported();
}
