tomcat/tomcat-9-CVE-2023-46589.patch
Michele Bussolotto 0dc13b7f1f Accepting request 1139478 from home:mbussolotto:branches:Java:packages
- Fixed CVEs:
  * CVE-2023-46589: Apache Tomcat: HTTP request smuggling due to
    incorrect headers parsing (bsc#1217649)
- Added patches:
  * tomcat-9-CVE-2023-46589.patch

OBS-URL: https://build.opensuse.org/request/show/1139478
OBS-URL: https://build.opensuse.org/package/show/Java:packages/tomcat?expand=0&rev=285
2024-01-17 14:32:35 +00:00

316 lines
13 KiB
Diff

Index: apache-tomcat-9.0.82-src/java/org/apache/catalina/connector/InputBuffer.java
===================================================================
--- apache-tomcat-9.0.82-src.orig/java/org/apache/catalina/connector/InputBuffer.java
+++ apache-tomcat-9.0.82-src/java/org/apache/catalina/connector/InputBuffer.java
@@ -29,6 +29,7 @@ import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import javax.servlet.ReadListener;
+import javax.servlet.RequestDispatcher;
import org.apache.catalina.security.SecurityUtil;
import org.apache.coyote.ActionCode;
@@ -295,6 +296,7 @@ public class InputBuffer extends Reader
*
* @throws IOException An underlying IOException occurred
*/
+ @SuppressWarnings("deprecation")
@Override
public int realReadBytes() throws IOException {
if (closed) {
@@ -307,10 +309,24 @@ public class InputBuffer extends Reader
try {
return coyoteRequest.doRead(this);
+ } 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);
- // An IOException on a read is almost always due to
- // the remote client aborting the request.
+ // 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);
}
}
Index: apache-tomcat-9.0.82-src/java/org/apache/catalina/connector/ClientAbortException.java
===================================================================
--- apache-tomcat-9.0.82-src.orig/java/org/apache/catalina/connector/ClientAbortException.java
+++ apache-tomcat-9.0.82-src/java/org/apache/catalina/connector/ClientAbortException.java
@@ -16,14 +16,12 @@
*/
package org.apache.catalina.connector;
-import java.io.IOException;
-
/**
* Extend IOException to identify 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-9.0.82-src/java/org/apache/catalina/core/ApplicationDispatcher.java
===================================================================
--- apache-tomcat-9.0.82-src.orig/java/org/apache/catalina/core/ApplicationDispatcher.java
+++ apache-tomcat-9.0.82-src/java/org/apache/catalina/core/ApplicationDispatcher.java
@@ -41,7 +41,7 @@ import org.apache.catalina.AsyncDispatch
import org.apache.catalina.Context;
import org.apache.catalina.Globals;
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;
@@ -661,7 +661,7 @@ final class ApplicationDispatcher implem
filterChain.doFilter(request, response);
}
// Servlet Service Method is called by the FilterChain
- } catch (ClientAbortException e) {
+ } catch (BadRequestException e) {
ioException = e;
} catch (IOException e) {
wrapper.getLogger().error(sm.getString("applicationDispatcher.serviceException", wrapper.getName()), e);
@@ -672,7 +672,7 @@ final class ApplicationDispatcher implem
wrapper.unavailable(e);
} catch (ServletException e) {
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-9.0.82-src/java/org/apache/catalina/core/StandardWrapperValve.java
===================================================================
--- apache-tomcat-9.0.82-src.orig/java/org/apache/catalina/core/StandardWrapperValve.java
+++ apache-tomcat-9.0.82-src/java/org/apache/catalina/core/StandardWrapperValve.java
@@ -32,7 +32,7 @@ import org.apache.catalina.Container;
import org.apache.catalina.Context;
import org.apache.catalina.Globals;
import org.apache.catalina.LifecycleException;
-import org.apache.catalina.connector.ClientAbortException;
+import org.apache.catalina.connector.BadRequestException;
import org.apache.catalina.connector.Request;
import org.apache.catalina.connector.Response;
import org.apache.catalina.valves.ValveBase;
@@ -170,7 +170,7 @@ final class StandardWrapperValve extends
}
}
- } catch (ClientAbortException | CloseNowException e) {
+ } catch (BadRequestException | CloseNowException e) {
if (container.getLogger().isDebugEnabled()) {
container.getLogger().debug(
sm.getString("standardWrapper.serviceException", wrapper.getName(), context.getName()), e);
@@ -191,7 +191,7 @@ final class StandardWrapperValve extends
// do not want to do exception(request, response, e) processing
} catch (ServletException e) {
Throwable rootCause = StandardWrapper.getRootCause(e);
- if (!(rootCause instanceof ClientAbortException)) {
+ if (!(rootCause instanceof BadRequestException)) {
container.getLogger().error(sm.getString("standardWrapper.serviceExceptionRoot", wrapper.getName(),
context.getName(), e.getMessage()), rootCause);
}
Index: apache-tomcat-9.0.82-src/java/org/apache/catalina/connector/BadRequestException.java
===================================================================
--- /dev/null
+++ apache-tomcat-9.0.82-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-9.0.82-src/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
===================================================================
--- apache-tomcat-9.0.82-src.orig/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
+++ apache-tomcat-9.0.82-src/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
@@ -428,6 +428,83 @@ 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-9.0.82-src/webapps/docs/changelog.xml
===================================================================
--- apache-tomcat-9.0.82-src.orig/webapps/docs/changelog.xml
+++ apache-tomcat-9.0.82-src/webapps/docs/changelog.xml
@@ -120,7 +120,7 @@
use of fully qualified class names in 9.0.81 that broke the jdbc-pool.
(markt)
</fix>
- </changelog>
+ </changelog>
</subsection>
</section>
<section name="Tomcat 9.0.81 (remm)" rtext="2023-10-10">
@@ -148,6 +148,11 @@
Improve handling of failures within <code>recycle()</code> methods.
(markt)
</add>
+ <fix>
+ Ensure that an <code>IOException</code> during the reading of the
+ request triggers always error handling, regardless of whether the
+ application swallows the exception. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">