--- java/org/apache/coyote/http11/AbstractHttp11Processor.java.orig 2014-03-17 16:00:40.592415000 -0400 +++ java/org/apache/coyote/http11/AbstractHttp11Processor.java 2014-03-18 13:39:06.789696000 -0400 @@ -684,13 +684,14 @@ /** * Initialize standard input and output filters. */ - protected void initializeFilters(int maxTrailerSize) { + protected void initializeFilters(int maxTrailerSize, int maxExtensionSize) { // Create and add the identity filters. getInputBuffer().addFilter(new IdentityInputFilter()); getOutputBuffer().addFilter(new IdentityOutputFilter()); // Create and add the chunked filters. - getInputBuffer().addFilter(new ChunkedInputFilter(maxTrailerSize)); + getInputBuffer().addFilter( + new ChunkedInputFilter(maxTrailerSize, maxExtensionSize)); getOutputBuffer().addFilter(new ChunkedOutputFilter()); // Create and add the void filters. --- java/org/apache/coyote/http11/AbstractHttp11Protocol.java.orig 2014-03-17 16:00:57.458467000 -0400 +++ java/org/apache/coyote/http11/AbstractHttp11Protocol.java 2014-03-17 16:40:11.035409000 -0400 @@ -151,7 +151,15 @@ this.maxTrailerSize = maxTrailerSize; } - + /** + * Maximum size of extension information in chunked encoding + */ + private int maxExtensionSize = 8192; + public int getMaxExtensionSize() { return maxExtensionSize; } + public void setMaxExtensionSize(int maxExtensionSize) { + this.maxExtensionSize = maxExtensionSize; + } + /** * This field indicates if the protocol is treated as if it is secure. This * normally means https is being used but can be used to fake https e.g --- java/org/apache/coyote/http11/Http11AprProcessor.java.orig 2014-03-17 16:01:22.889559000 -0400 +++ java/org/apache/coyote/http11/Http11AprProcessor.java 2014-03-17 16:43:14.716027000 -0400 @@ -58,7 +58,7 @@ public Http11AprProcessor(int headerBufferSize, AprEndpoint endpoint, - int maxTrailerSize) { + int maxTrailerSize, int maxExtensionSize) { super(endpoint); @@ -68,7 +68,7 @@ outputBuffer = new InternalAprOutputBuffer(response, headerBufferSize); response.setOutputBuffer(outputBuffer); - initializeFilters(maxTrailerSize); + initializeFilters(maxTrailerSize, maxExtensionSize); } --- java/org/apache/coyote/http11/Http11AprProtocol.java.orig 2014-03-17 16:10:16.268358000 -0400 +++ java/org/apache/coyote/http11/Http11AprProtocol.java 2014-03-17 16:50:17.428466000 -0400 @@ -294,7 +294,7 @@ protected Http11AprProcessor createProcessor() { Http11AprProcessor processor = new Http11AprProcessor( proto.getMaxHttpHeaderSize(), (AprEndpoint)proto.endpoint, - proto.getMaxTrailerSize()); + proto.getMaxTrailerSize(), proto.getMaxExtensionSize()); processor.setAdapter(proto.adapter); processor.setMaxKeepAliveRequests(proto.getMaxKeepAliveRequests()); processor.setKeepAliveTimeout(proto.getKeepAliveTimeout()); --- java/org/apache/coyote/http11/Http11NioProcessor.java.orig 2014-03-17 16:02:20.016748000 -0400 +++ java/org/apache/coyote/http11/Http11NioProcessor.java 2014-03-17 16:51:55.623782000 -0400 @@ -63,7 +63,7 @@ public Http11NioProcessor(int maxHttpHeaderSize, NioEndpoint endpoint, - int maxTrailerSize) { + int maxTrailerSize, int maxExtensionSize) { super(endpoint); @@ -73,7 +73,7 @@ outputBuffer = new InternalNioOutputBuffer(response, maxHttpHeaderSize); response.setOutputBuffer(outputBuffer); - initializeFilters(maxTrailerSize); + initializeFilters(maxTrailerSize, maxExtensionSize); } --- java/org/apache/coyote/http11/Http11NioProtocol.java.orig 2014-03-17 16:07:26.027787000 -0400 +++ java/org/apache/coyote/http11/Http11NioProtocol.java 2014-03-17 16:53:09.198025000 -0400 @@ -260,7 +260,7 @@ public Http11NioProcessor createProcessor() { Http11NioProcessor processor = new Http11NioProcessor( proto.getMaxHttpHeaderSize(), (NioEndpoint)proto.endpoint, - proto.getMaxTrailerSize()); + proto.getMaxTrailerSize(), proto.getMaxExtensionSize()); processor.setAdapter(proto.adapter); processor.setMaxKeepAliveRequests(proto.getMaxKeepAliveRequests()); processor.setKeepAliveTimeout(proto.getKeepAliveTimeout()); --- java/org/apache/coyote/http11/Http11Processor.java.orig 2014-03-17 16:07:45.099837000 -0400 +++ java/org/apache/coyote/http11/Http11Processor.java 2014-03-18 12:42:34.018260000 -0400 @@ -50,7 +50,7 @@ public Http11Processor(int headerBufferSize, JIoEndpoint endpoint, - int maxTrailerSize) { + int maxTrailerSize, int maxExtensionSize) { super(endpoint); @@ -60,7 +60,7 @@ outputBuffer = new InternalOutputBuffer(response, headerBufferSize); response.setOutputBuffer(outputBuffer); - initializeFilters(maxTrailerSize); + initializeFilters(maxTrailerSize, maxExtensionSize); } --- java/org/apache/coyote/http11/Http11Protocol.java.orig 2014-03-17 16:08:00.058113000 -0400 +++ java/org/apache/coyote/http11/Http11Protocol.java 2014-03-17 16:56:04.194609000 -0400 @@ -164,7 +164,7 @@ protected Http11Processor createProcessor() { Http11Processor processor = new Http11Processor( proto.getMaxHttpHeaderSize(), (JIoEndpoint)proto.endpoint, - proto.getMaxTrailerSize()); + proto.getMaxTrailerSize(), proto.getMaxExtensionSize()); processor.setAdapter(proto.adapter); processor.setMaxKeepAliveRequests(proto.getMaxKeepAliveRequests()); processor.setKeepAliveTimeout(proto.getKeepAliveTimeout()); --- java/org/apache/coyote/http11/filters/ChunkedInputFilter.java.orig 2014-03-17 16:08:12.213985000 -0400 +++ java/org/apache/coyote/http11/filters/ChunkedInputFilter.java 2014-04-15 19:51:43.729201000 -0400 @@ -39,6 +39,8 @@ */ public class ChunkedInputFilter implements InputFilter { + private static final org.apache.juli.logging.Log log + = org.apache.juli.logging.LogFactory.getLog(ChunkedInputFilter.class); // -------------------------------------------------------------- Constants @@ -118,9 +120,29 @@ */ private Request request; + + /** + * Limit for extension size. + */ + private final long maxExtensionSize; + + + /** + * Limit for trailer size. + */ + private int maxTrailerSize; + + + /** + * Size of extensions processed for this request. + */ + private long extensionSize; + // ----------------------------------------------------------- Constructors - public ChunkedInputFilter(int maxTrailerSize) { + public ChunkedInputFilter(int maxTrailerSize, int maxExtensionSize) { this.trailingHeaders.setLimit(maxTrailerSize); + this.maxTrailerSize = maxTrailerSize; + this.maxExtensionSize = maxExtensionSize; } // ---------------------------------------------------- InputBuffer Methods @@ -250,6 +272,8 @@ endChunk = false; needCRLFParse = false; trailingHeaders.recycle(); + trailingHeaders.setLimit(maxTrailerSize); + extensionSize = 0; } @@ -299,7 +323,7 @@ int result = 0; boolean eol = false; boolean readDigit = false; - boolean trailer = false; + boolean extension = false; while (!eol) { @@ -309,11 +333,17 @@ } if (buf[pos] == Constants.CR || buf[pos] == Constants.LF) { + log.info("EOL is true"); parseCRLF(false); eol = true; - } else if (buf[pos] == Constants.SEMI_COLON) { - trailer = true; - } else if (!trailer) { + } else if (buf[pos] == Constants.SEMI_COLON && !extension) { + // First semi-colon marks the start of the extension. Further + // semi-colons may appear to separate multiple chunk-extensions. + // These need to be processed as part of parsing the extensions. + extension = true; + extensionSize++; + log.info("SEMI_COLON"); + } else if (!extension) { //don't read data after the trailer int charValue = HexUtils.getDec(buf[pos]); if (charValue != -1) { @@ -323,15 +353,22 @@ } else { //we shouldn't allow invalid, non hex characters //in the chunked header + log.info("Returning false"); return false; } - } - + } else { + // Extension 'parsing' + // Note that the chunk-extension is neither parsed nor + // validated. Currently it is simply ignored. + extensionSize++; + if (maxExtensionSize > -1 && extensionSize > maxExtensionSize) { + throw new IOException("maxExtensionSize exceeded"); + } + } // Parsing the CRLF increments pos if (!eol) { pos++; } - } if (!readDigit) @@ -489,12 +526,17 @@ chr = buf[pos]; if ((chr == Constants.SP) || (chr == Constants.HT)) { pos++; + // If we swallow whitespace, make sure it counts towards the + // limit placed on trailing header size + int newlimit = trailingHeaders.getLimit() -1; + if (trailingHeaders.getEnd() > newlimit) { + throw new IOException("Exceeded maxTrailerSize"); + } + trailingHeaders.setLimit(newlimit); } else { space = false; } - } - // Reading bytes until the end of the line while (!eol) { --- test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java.orig 2014-03-17 16:08:33.031999000 -0400 +++ test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java 2014-03-17 17:40:23.853592000 -0400 @@ -41,6 +41,7 @@ public class TestChunkedInputFilter extends TomcatBaseTest { private static final String LF = "\n"; + private static final int EXT_SIZE_LIMIT = 10; @Test public void testChunkHeaderCRLF() throws Exception { @@ -202,6 +203,79 @@ assertTrue(client.isResponse500()); } + + @Test + public void testExtensionSizeLimitOneBelow() throws Exception { + doTestExtensionSizeLimit(EXT_SIZE_LIMIT - 1, true); + } + + + @Test + public void testExtensionSizeLimitExact() throws Exception { + doTestExtensionSizeLimit(EXT_SIZE_LIMIT, true); + } + + + @Test + public void testExtensionSizeLimitOneOver() throws Exception { + doTestExtensionSizeLimit(EXT_SIZE_LIMIT + 1, false); + } + + private void doTestExtensionSizeLimit(int len, boolean ok) + throws Exception { + // Setup Tomcat instance + Tomcat tomcat = getTomcatInstance(); + + tomcat.getConnector().setProperty( + "maxExtensionSize", Integer.toString(EXT_SIZE_LIMIT)); + + // Must have a real docBase - just use temp + Context ctx = + tomcat.addContext("", System.getProperty("java.io.tmpdir")); + + Tomcat.addServlet(ctx, "servlet", new EchoHeaderServlet()); + ctx.addServletMapping("/", "servlet"); + + tomcat.start(); + + String extName = ";foo="; + StringBuilder extValue = new StringBuilder(len); + for (int i = 0; i < (len - extName.length()); i++) { + extValue.append("x"); + } + + String[] request = new String[]{ + "POST /echo-params.jsp HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: any" + SimpleHttpClient.CRLF + + "Transfer-encoding: chunked" + SimpleHttpClient.CRLF + + "Content-Type: application/x-www-form-urlencoded" + + SimpleHttpClient.CRLF + + "Connection: close" + SimpleHttpClient.CRLF + + SimpleHttpClient.CRLF + + "3" + extName + extValue.toString() + SimpleHttpClient.CRLF + + "a=0" + SimpleHttpClient.CRLF + + "4" + SimpleHttpClient.CRLF + + "&b=1" + SimpleHttpClient.CRLF + + "0" + SimpleHttpClient.CRLF + + SimpleHttpClient.CRLF }; + + TrailerClient client = + new TrailerClient(tomcat.getConnector().getLocalPort()); + client.setRequest(request); + + client.connect(); + client.processRequest(); + + if (ok) { + assertTrue(client.isResponse200()); + } else { + assertTrue(client.isResponse500()); + } + } + + + + @Test public void testNoTrailingHeaders() throws Exception { // Setup Tomcat instance --- webapps/docs/changelog.xml.orig 2014-03-17 16:08:46.095050000 -0400 +++ webapps/docs/changelog.xml 2014-03-17 17:44:14.163385000 -0400 @@ -394,6 +394,11 @@
+ + Add support for limiting the size of chunk extensions when using chunked + encoding. (markt) + CVE-2013-4322 patch applied by Red Hat. + Update Tomcat's internal copy of Commons FileUpload to FileUpload 1.3. (markt) --- webapps/docs/config/http.xml.orig 2014-03-17 16:08:59.013101000 -0400 +++ webapps/docs/config/http.xml 2014-03-17 18:10:13.965639000 -0400 @@ -399,6 +399,12 @@ and connections are not counted.

+ +

Limits the total length of chunk extensions in chunked HTTP requests. + If the value is -1, no limit will be imposed. If not + specified, the default value of 8192 will be used.

+
+

The maximum size of the request and response HTTP header, specified in bytes. If not specified, this attribute is set to 8192 (8 KB).