tomcat/CVE-2019-0199-2.patch

247 lines
12 KiB
Diff
Raw Normal View History

2020-02-28 20:54:21 -05:00
diff -Nurp apache-tomcat-9.0.10-src/java/org/apache/catalina/connector/OutputBuffer.java apache-tomcat-9.0.10-src-bak/java/org/apache/catalina/connector/OutputBuffer.java
--- apache-tomcat-9.0.10-src/java/org/apache/catalina/connector/OutputBuffer.java 2018-06-20 13:35:33.000000000 -0400
+++ apache-tomcat-9.0.10-src-bak/java/org/apache/catalina/connector/OutputBuffer.java 2019-06-09 20:28:02.836000000 -0400
@@ -33,6 +33,7 @@ import javax.servlet.http.HttpServletRes
import org.apache.catalina.Globals;
import org.apache.coyote.ActionCode;
+import org.apache.coyote.CloseNowException;
import org.apache.coyote.Response;
import org.apache.tomcat.util.buf.C2BConverter;
import org.apache.tomcat.util.res.StringManager;
@@ -326,6 +327,13 @@ public class OutputBuffer extends Writer
// real write to the adapter
try {
coyoteResponse.doWrite(buf);
+ } catch (CloseNowException e) {
+ // Catch this sub-class as it requires specific handling.
+ // Examples where this exception is thrown:
+ // - HTTP/2 stream timeout
+ // Prevent further output for this response
+ closed = true;
+ throw e;
} catch (IOException e) {
// An IOException on a write is almost always due to
// the remote client aborting the request. Wrap this
diff -Nurp apache-tomcat-9.0.10-src/java/org/apache/catalina/core/StandardWrapperValve.java apache-tomcat-9.0.10-src-bak/java/org/apache/catalina/core/StandardWrapperValve.java
--- apache-tomcat-9.0.10-src/java/org/apache/catalina/core/StandardWrapperValve.java 2018-06-20 13:35:34.000000000 -0400
+++ apache-tomcat-9.0.10-src-bak/java/org/apache/catalina/core/StandardWrapperValve.java 2019-06-09 20:33:27.596000000 -0400
@@ -36,6 +36,7 @@ import org.apache.catalina.connector.Cli
import org.apache.catalina.connector.Request;
import org.apache.catalina.connector.Response;
import org.apache.catalina.valves.ValveBase;
+import org.apache.coyote.CloseNowException;
import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.buf.MessageBytes;
import org.apache.tomcat.util.log.SystemLogHandler;
@@ -201,7 +202,7 @@ final class StandardWrapperValve
}
}
- } catch (ClientAbortException e) {
+ } catch (ClientAbortException | CloseNowException e) {
throwable = e;
exception(request, response, e);
} catch (IOException e) {
diff -Nurp apache-tomcat-9.0.10-src/java/org/apache/coyote/http2/LocalStrings.properties apache-tomcat-9.0.10-src-bak/java/org/apache/coyote/http2/LocalStrings.properties
--- apache-tomcat-9.0.10-src/java/org/apache/coyote/http2/LocalStrings.properties 2018-06-20 13:35:35.000000000 -0400
+++ apache-tomcat-9.0.10-src-bak/java/org/apache/coyote/http2/LocalStrings.properties 2019-06-09 20:34:32.307000000 -0400
@@ -93,6 +93,7 @@ stream.reset.fail=Connection [{0}], Stre
stream.reset.receive=Connection [{0}], Stream [{1}], Reset received due to [{2}]
stream.reset.send=Connection [{0}], Stream [{1}], Reset sent due to [{2}]
stream.trailerHeader.noEndOfStream=Connection [{0}], Stream [{1}], The trailer headers did not include the end of stream flag
+stream.writeTimeout=Timeout waiting for client to increase flow control window to permit stream data to be written
stream.inputBuffer.copy=Copying [{0}] bytes from inBuffer to outBuffer
stream.inputBuffer.dispatch=Data added to inBuffer when read interest is registered. Triggering a read dispatch
@@ -149,4 +150,4 @@ upgradeHandler.writeHeaders=Connection [
upgradeHandler.writePushHeaders=Connection [{0}], Stream [{1}], Pushed stream [{2}], EndOfStream [{3}]
writeStateMachine.endWrite.ise=It is illegal to specify [{0}] for the new state once a write has completed
-writeStateMachine.ise=It is illegal to call [{0}()] in state [{1}]
\ No newline at end of file
+writeStateMachine.ise=It is illegal to call [{0}()] in state [{1}]
diff -Nurp apache-tomcat-9.0.10-src/java/org/apache/coyote/http2/Stream.java apache-tomcat-9.0.10-src-bak/java/org/apache/coyote/http2/Stream.java
--- apache-tomcat-9.0.10-src/java/org/apache/coyote/http2/Stream.java 2018-06-20 13:35:35.000000000 -0400
+++ apache-tomcat-9.0.10-src-bak/java/org/apache/coyote/http2/Stream.java 2019-06-09 20:38:30.109000000 -0400
@@ -211,7 +211,21 @@ class Stream extends AbstractStream impl
}
try {
if (block) {
- wait();
+ wait(handler.getProtocol().getStreamWriteTimeout());
+ windowSize = getWindowSize();
+ if (windowSize == 0) {
+ String msg = sm.getString("stream.writeTimeout");
+ StreamException se = new StreamException(
+ msg, Http2Error.ENHANCE_YOUR_CALM, getIdAsInt());
+ // Prevent the application making further writes
+ streamOutputBuffer.closed = true;
+ // Prevent Tomcat's error handling trying to write
+ coyoteResponse.setError();
+ coyoteResponse.setErrorReported();
+ // Trigger a reset once control returns to Tomcat
+ streamOutputBuffer.reset = se;
+ throw new CloseNowException(msg, se);
+ }
} else {
return 0;
}
@@ -221,7 +235,6 @@ class Stream extends AbstractStream impl
// Stream.
throw new IOException(e);
}
- windowSize = getWindowSize();
}
int allocation;
if (windowSize < reservation) {
@@ -660,6 +673,9 @@ class Stream extends AbstractStream impl
return !streamOutputBuffer.endOfStreamSent;
}
+ StreamException getResetException() {
+ return streamOutputBuffer.reset;
+ }
private static void push(final Http2UpgradeHandler handler, final Request request,
final Stream stream) throws IOException {
@@ -707,6 +723,7 @@ class Stream extends AbstractStream impl
private final ByteBuffer buffer = ByteBuffer.allocate(8 * 1024);
private volatile long written = 0;
private volatile boolean closed = false;
+ private volatile StreamException reset = null;
private volatile boolean endOfStreamSent = false;
/* The write methods are synchronized to ensure that only one thread at
@@ -800,9 +817,14 @@ class Stream extends AbstractStream impl
@Override
public final void end() throws IOException {
- closed = true;
- flush(true);
- writeTrailers();
+ if (reset != null) {
+ throw new CloseNowException(reset);
+ }
+ if (!closed) {
+ closed = true;
+ flush(true);
+ writeTrailers();
+ }
}
/**
diff -Nurp apache-tomcat-9.0.10-src/java/org/apache/coyote/http2/StreamProcessor.java apache-tomcat-9.0.10-src-bak/java/org/apache/coyote/http2/StreamProcessor.java
--- apache-tomcat-9.0.10-src/java/org/apache/coyote/http2/StreamProcessor.java 2018-06-20 13:35:35.000000000 -0400
+++ apache-tomcat-9.0.10-src-bak/java/org/apache/coyote/http2/StreamProcessor.java 2019-06-09 20:40:08.789000000 -0400
@@ -78,10 +78,13 @@ class StreamProcessor extends AbstractPr
stream.getIdentifier()), Http2Error.INTERNAL_ERROR);
stream.close(ce);
} else if (!getErrorState().isIoAllowed()) {
- StreamException se = new StreamException(sm.getString(
- "streamProcessor.error.stream", stream.getConnectionId(),
- stream.getIdentifier()), Http2Error.INTERNAL_ERROR,
- stream.getIdentifier().intValue());
+ StreamException se = stream.getResetException();
+ if (se == null) {
+ se = new StreamException(sm.getString(
+ "streamProcessor.error.stream", stream.getConnectionId(),
+ stream.getIdentifier()), Http2Error.INTERNAL_ERROR,
+ stream.getIdentifier().intValue());
+ }
stream.close(se);
}
}
diff -Nurp apache-tomcat-9.0.10-src/test/org/apache/coyote/http2/Http2TestBase.java apache-tomcat-9.0.10-src-bak/test/org/apache/coyote/http2/Http2TestBase.java
--- apache-tomcat-9.0.10-src/test/org/apache/coyote/http2/Http2TestBase.java 2018-06-20 13:35:38.000000000 -0400
+++ apache-tomcat-9.0.10-src-bak/test/org/apache/coyote/http2/Http2TestBase.java 2019-06-09 20:41:45.113000000 -0400
@@ -486,8 +486,10 @@ public abstract class Http2TestBase exte
Http2Protocol http2Protocol = new Http2Protocol();
// Short timeouts for now. May need to increase these for CI systems.
http2Protocol.setReadTimeout(2000);
- http2Protocol.setKeepAliveTimeout(5000);
http2Protocol.setWriteTimeout(2000);
+ http2Protocol.setKeepAliveTimeout(5000);
+ http2Protocol.setStreamReadTimeout(1000);
+ http2Protocol.setStreamWriteTimeout(1000);
http2Protocol.setMaxConcurrentStreams(maxConcurrentStreams);
connector.addUpgradeProtocol(http2Protocol);
}
diff -Nurp apache-tomcat-9.0.10-src/test/org/apache/coyote/http2/TestHttp2Timeouts.java apache-tomcat-9.0.10-src-bak/test/org/apache/coyote/http2/TestHttp2Timeouts.java
--- apache-tomcat-9.0.10-src/test/org/apache/coyote/http2/TestHttp2Timeouts.java 1969-12-31 19:00:00.000000000 -0500
+++ apache-tomcat-9.0.10-src-bak/test/org/apache/coyote/http2/TestHttp2Timeouts.java 2019-06-09 20:42:38.095000000 -0400
@@ -0,0 +1,73 @@
+/*
+ * 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.coyote.http2;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+public class TestHttp2Timeouts extends Http2TestBase {
+
+ @Override
+ @Before
+ public void http2Connect() throws Exception {
+ super.http2Connect();
+ sendSettings(0, false, new SettingValue(Setting.INITIAL_WINDOW_SIZE.getId(), 0));
+ }
+
+
+ /*
+ * Simple request won't fill buffer so timeout will occur in Tomcat internal
+ * code during response completion.
+ */
+ @Test
+ public void testClientWithEmptyWindow() throws Exception {
+ sendSimpleGetRequest(3);
+
+ // Settings
+ parser.readFrame(false);
+ // Headers
+ parser.readFrame(false);
+
+ output.clearTrace();
+
+ parser.readFrame(false);
+ Assert.assertEquals("3-RST-[11]\n", output.getTrace());
+ }
+
+
+ /*
+ * Large request will fill buffer so timeout will occur in application code
+ * during response write (when Tomcat commits the response and flushes the
+ * buffer as a result of the buffer filling).
+ */
+ @Test
+ public void testClientWithEmptyWindowLargeResponse() throws Exception {
+ sendLargeGetRequest(3);
+
+ // Settings
+ parser.readFrame(false);
+ // Headers
+ parser.readFrame(false);
+
+ output.clearTrace();
+
+ parser.readFrame(false);
+ Assert.assertEquals("3-RST-[11]\n", output.getTrace());
+ }
+
+}