--- conf/catalina.properties.orig 2017-03-28 15:06:40.627389816 -0400 +++ conf/catalina.properties 2017-03-28 15:06:40.657389967 -0400 @@ -131,3 +131,7 @@ #tomcat.util.buf.StringCache.char.enabled=true #tomcat.util.buf.StringCache.trainThreshold=500000 #tomcat.util.buf.StringCache.cacheSize=5000 + +# Allow for changes to HTTP request validation +# WARNING: Using this option will expose the server to CVE-2016-6816 +#tomcat.util.http.parser.HttpParser.requestTargetAllow=| --- java/org/apache/coyote/http11/AbstractInputBuffer.java.orig 2017-03-28 15:06:40.628389821 -0400 +++ java/org/apache/coyote/http11/AbstractInputBuffer.java 2017-03-28 15:06:40.649389926 -0400 @@ -28,62 +28,10 @@ public abstract class AbstractInputBuffer implements InputBuffer{ - protected static final boolean[] HTTP_TOKEN_CHAR = new boolean[128]; - /** * The string manager for this package. */ - protected static final StringManager sm = - StringManager.getManager(Constants.Package); - - - static { - for (int i = 0; i < 128; i++) { - if (i < 32) { - HTTP_TOKEN_CHAR[i] = false; - } else if (i == 127) { - HTTP_TOKEN_CHAR[i] = false; - } else if (i == '(') { - HTTP_TOKEN_CHAR[i] = false; - } else if (i == ')') { - HTTP_TOKEN_CHAR[i] = false; - } else if (i == '<') { - HTTP_TOKEN_CHAR[i] = false; - } else if (i == '>') { - HTTP_TOKEN_CHAR[i] = false; - } else if (i == '@') { - HTTP_TOKEN_CHAR[i] = false; - } else if (i == ',') { - HTTP_TOKEN_CHAR[i] = false; - } else if (i == ';') { - HTTP_TOKEN_CHAR[i] = false; - } else if (i == ':') { - HTTP_TOKEN_CHAR[i] = false; - } else if (i == '\\') { - HTTP_TOKEN_CHAR[i] = false; - } else if (i == '\"') { - HTTP_TOKEN_CHAR[i] = false; - } else if (i == '/') { - HTTP_TOKEN_CHAR[i] = false; - } else if (i == '[') { - HTTP_TOKEN_CHAR[i] = false; - } else if (i == ']') { - HTTP_TOKEN_CHAR[i] = false; - } else if (i == '?') { - HTTP_TOKEN_CHAR[i] = false; - } else if (i == '=') { - HTTP_TOKEN_CHAR[i] = false; - } else if (i == '{') { - HTTP_TOKEN_CHAR[i] = false; - } else if (i == '}') { - HTTP_TOKEN_CHAR[i] = false; - } else if (i == ' ') { - HTTP_TOKEN_CHAR[i] = false; - } else { - HTTP_TOKEN_CHAR[i] = true; - } - } - } + protected static final StringManager sm = StringManager.getManager(Constants.Package); /** --- java/org/apache/coyote/http11/InternalAprInputBuffer.java.orig 2017-03-28 15:06:40.629389826 -0400 +++ java/org/apache/coyote/http11/InternalAprInputBuffer.java 2017-03-28 15:06:40.650389932 -0400 @@ -30,6 +30,7 @@ import org.apache.tomcat.jni.Status; import org.apache.tomcat.util.buf.ByteChunk; import org.apache.tomcat.util.buf.MessageBytes; +import org.apache.tomcat.util.http.parser.HttpParser; import org.apache.tomcat.util.net.AbstractEndpoint; import org.apache.tomcat.util.net.SocketWrapper; @@ -70,7 +71,7 @@ parsingHeader = true; swallowInput = true; - + } @@ -93,7 +94,7 @@ // --------------------------------------------------------- Public Methods /** - * Recycle the input buffer. This should be called when closing the + * Recycle the input buffer. This should be called when closing the * connection. */ @Override @@ -105,14 +106,14 @@ /** - * Read the request line. This function is meant to be used during the - * HTTP request header parsing. Do NOT attempt to read the request body + * Read the request line. This function is meant to be used during the + * HTTP request header parsing. Do NOT attempt to read the request body * using it. * * @throws IOException If an exception occurs during the underlying socket * read operations, or if the given buffer is not big enough to accommodate * the whole line. - * @return true if data is properly fed; false if no data is available + * @return true if data is properly fed; false if no data is available * immediately and thread should be freed */ @Override @@ -159,7 +160,7 @@ // // Reading the method name - // Method name is always US-ASCII + // Method name is a token // boolean space = false; @@ -172,22 +173,20 @@ throw new EOFException(sm.getString("iib.eof.error")); } - // Spec says no CR or LF in method name - if (buf[pos] == Constants.CR || buf[pos] == Constants.LF) { - throw new IllegalArgumentException( - sm.getString("iib.invalidmethod")); - } - // Spec says single SP but it also says be tolerant of HT + // Spec says method name is a token followed by a single SP but + // also be tolerant of multiple SP and/or HT. if (buf[pos] == Constants.SP || buf[pos] == Constants.HT) { space = true; request.method().setBytes(buf, start, pos - start); + } else if (!HttpParser.isToken(buf[pos])) { + throw new IllegalArgumentException(sm.getString("iib.invalidmethod")); } pos++; } - // Spec says single SP but also says be tolerant of multiple and/or HT + // Spec says single SP but also says be tolerant of multiple SP and/or HT while (space) { // Read new bytes if needed if (pos >= lastValid) { @@ -224,15 +223,16 @@ if (buf[pos] == Constants.SP || buf[pos] == Constants.HT) { space = true; end = pos; - } else if ((buf[pos] == Constants.CR) + } else if ((buf[pos] == Constants.CR) || (buf[pos] == Constants.LF)) { // HTTP/0.9 style request eol = true; space = true; end = pos; - } else if ((buf[pos] == Constants.QUESTION) - && (questionPos == -1)) { + } else if ((buf[pos] == Constants.QUESTION) && (questionPos == -1)) { questionPos = pos; + } else if (HttpParser.isNotRequestTarget(buf[pos])) { + throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget")); } pos++; @@ -241,7 +241,7 @@ request.unparsedURI().setBytes(buf, start, end - start); if (questionPos >= 0) { - request.queryString().setBytes(buf, questionPos + 1, + request.queryString().setBytes(buf, questionPos + 1, end - questionPos - 1); request.requestURI().setBytes(buf, start, questionPos - start); } else { @@ -269,7 +269,7 @@ // // Reading the protocol - // Protocol is always US-ASCII + // Protocol is always "HTTP/" DIGIT "." DIGIT // while (!eol) { @@ -286,6 +286,8 @@ if (end == 0) end = pos; eol = true; + } else if (!HttpParser.isHttpProtocol(buf[pos])) { + throw new IllegalArgumentException(sm.getString("iib.invalidHttpProtocol")); } pos++; @@ -297,7 +299,7 @@ } else { request.protocol().setString(""); } - + return true; } @@ -326,7 +328,7 @@ /** * Parse an HTTP header. - * + * * @return false after reading a blank line (which indicates that the * HTTP header parsing is done */ @@ -384,7 +386,7 @@ if (buf[pos] == Constants.COLON) { colon = true; headerValue = headers.addValue(buf, start, pos - start); - } else if (!HTTP_TOKEN_CHAR[buf[pos]]) { + } else if (!HttpParser.isToken(buf[pos])) { // If a non-token header is detected, skip the line and // ignore the header skipLine(start); @@ -490,14 +492,14 @@ } - + private void skipLine(int start) throws IOException { boolean eol = false; int lastRealByte = start; if (pos - 1 > start) { lastRealByte = pos - 1; } - + while (!eol) { // Read new bytes if needed @@ -521,8 +523,8 @@ lastRealByte - start + 1, Charset.forName("ISO-8859-1")))); } } - - + + // ---------------------------------------------------- InputBuffer Methods @@ -530,7 +532,7 @@ * Read some bytes. */ @Override - public int doRead(ByteChunk chunk, Request req) + public int doRead(ByteChunk chunk, Request req) throws IOException { if (lastActiveFilter == -1) @@ -558,11 +560,11 @@ // Ignore the block parameter and just call fill return fill(); } - - + + /** * Fill the internal buffer using data from the underlying input stream. - * + * * @return false if at end of stream */ protected boolean fill() @@ -594,7 +596,7 @@ } else { if (buf.length - end < 4500) { - // In this case, the request header was really large, so we allocate a + // In this case, the request header was really large, so we allocate a // brand new one; the old one will get GCed when subsequent requests // clear all references buf = new byte[buf.length]; @@ -640,7 +642,7 @@ * This class is an input buffer which will read its data from an input * stream. */ - protected class SocketInputBuffer + protected class SocketInputBuffer implements InputBuffer { @@ -648,7 +650,7 @@ * Read bytes into the specified chunk. */ @Override - public int doRead(ByteChunk chunk, Request req ) + public int doRead(ByteChunk chunk, Request req ) throws IOException { if (pos >= lastValid) { --- java/org/apache/coyote/http11/InternalInputBuffer.java.orig 2017-03-28 15:06:40.630389831 -0400 +++ java/org/apache/coyote/http11/InternalInputBuffer.java 2017-03-28 15:06:40.650389932 -0400 @@ -28,6 +28,7 @@ import org.apache.juli.logging.LogFactory; import org.apache.tomcat.util.buf.ByteChunk; import org.apache.tomcat.util.buf.MessageBytes; +import org.apache.tomcat.util.http.parser.HttpParser; import org.apache.tomcat.util.net.AbstractEndpoint; import org.apache.tomcat.util.net.SocketWrapper; @@ -69,10 +70,10 @@ } - + /** - * Read the request line. This function is meant to be used during the - * HTTP request header parsing. Do NOT attempt to read the request body + * Read the request line. This function is meant to be used during the + * HTTP request header parsing. Do NOT attempt to read the request body * using it. * * @throws IOException If an exception occurs during the underlying socket @@ -81,7 +82,7 @@ */ @Override public boolean parseRequestLine(boolean useAvailableDataOnly) - + throws IOException { int start = 0; @@ -113,7 +114,7 @@ // // Reading the method name - // Method name is always US-ASCII + // Method name is a token // boolean space = false; @@ -126,23 +127,20 @@ throw new EOFException(sm.getString("iib.eof.error")); } - // Spec says no CR or LF in method name - if (buf[pos] == Constants.CR || buf[pos] == Constants.LF) { - throw new IllegalArgumentException( - sm.getString("iib.invalidmethod")); - } - // Spec says single SP but it also says be tolerant of HT + // Spec says method name is a token followed by a single SP but + // also be tolerant of multiple SP and/or HT. if (buf[pos] == Constants.SP || buf[pos] == Constants.HT) { space = true; request.method().setBytes(buf, start, pos - start); + } else if (!HttpParser.isToken(buf[pos])) { + throw new IllegalArgumentException(sm.getString("iib.invalidmethod")); } pos++; } - - // Spec says single SP but also says be tolerant of multiple and/or HT + // Spec says single SP but also be tolerant of multiple SP and/or HT while (space) { // Read new bytes if needed if (pos >= lastValid) { @@ -179,15 +177,16 @@ if (buf[pos] == Constants.SP || buf[pos] == Constants.HT) { space = true; end = pos; - } else if ((buf[pos] == Constants.CR) + } else if ((buf[pos] == Constants.CR) || (buf[pos] == Constants.LF)) { // HTTP/0.9 style request eol = true; space = true; end = pos; - } else if ((buf[pos] == Constants.QUESTION) - && (questionPos == -1)) { + } else if ((buf[pos] == Constants.QUESTION) && (questionPos == -1)) { questionPos = pos; + } else if (HttpParser.isNotRequestTarget(buf[pos])) { + throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget")); } pos++; @@ -196,14 +195,14 @@ request.unparsedURI().setBytes(buf, start, end - start); if (questionPos >= 0) { - request.queryString().setBytes(buf, questionPos + 1, + request.queryString().setBytes(buf, questionPos + 1, end - questionPos - 1); request.requestURI().setBytes(buf, start, questionPos - start); } else { request.requestURI().setBytes(buf, start, end - start); } - // Spec says single SP but also says be tolerant of multiple and/or HT + // Spec says single SP but also says be tolerant of multiple SP and/or HT while (space) { // Read new bytes if needed if (pos >= lastValid) { @@ -223,9 +222,8 @@ // // Reading the protocol - // Protocol is always US-ASCII + // Protocol is always "HTTP/" DIGIT "." DIGIT // - while (!eol) { // Read new bytes if needed @@ -240,6 +238,8 @@ if (end == 0) end = pos; eol = true; + } else if (!HttpParser.isHttpProtocol(buf[pos])) { + throw new IllegalArgumentException(sm.getString("iib.invalidHttpProtocol")); } pos++; @@ -251,7 +251,7 @@ } else { request.protocol().setString(""); } - + return true; } @@ -280,7 +280,7 @@ /** * Parse an HTTP header. - * + * * @return false after reading a blank line (which indicates that the * HTTP header parsing is done */ @@ -338,7 +338,7 @@ if (buf[pos] == Constants.COLON) { colon = true; headerValue = headers.addValue(buf, start, pos - start); - } else if (!HTTP_TOKEN_CHAR[buf[pos]]) { + } else if (!HttpParser.isToken(buf[pos])) { // If a non-token header is detected, skip the line and // ignore the header skipLine(start); @@ -470,7 +470,7 @@ if (pos - 1 > start) { lastRealByte = pos - 1; } - + while (!eol) { // Read new bytes if needed @@ -497,7 +497,7 @@ /** * Fill the internal buffer using data from the underlying input stream. - * + * * @return false if at end of stream */ protected boolean fill() throws IOException { @@ -524,7 +524,7 @@ } else { if (buf.length - end < 4500) { - // In this case, the request header was really large, so we allocate a + // In this case, the request header was really large, so we allocate a // brand new one; the old one will get GCed when subsequent requests // clear all references buf = new byte[buf.length]; @@ -551,7 +551,7 @@ * This class is an input buffer which will read its data from an input * stream. */ - protected class InputStreamInputBuffer + protected class InputStreamInputBuffer implements InputBuffer { @@ -559,7 +559,7 @@ * Read bytes into the specified chunk. */ @Override - public int doRead(ByteChunk chunk, Request req ) + public int doRead(ByteChunk chunk, Request req ) throws IOException { if (pos >= lastValid) { --- java/org/apache/coyote/http11/InternalNioInputBuffer.java.orig 2017-03-28 15:06:40.631389836 -0400 +++ java/org/apache/coyote/http11/InternalNioInputBuffer.java 2017-03-28 15:06:40.650389932 -0400 @@ -25,6 +25,7 @@ import org.apache.coyote.Request; import org.apache.tomcat.util.buf.ByteChunk; import org.apache.tomcat.util.buf.MessageBytes; +import org.apache.tomcat.util.http.parser.HttpParser; import org.apache.tomcat.util.net.AbstractEndpoint; import org.apache.tomcat.util.net.NioChannel; import org.apache.tomcat.util.net.NioEndpoint; @@ -92,7 +93,7 @@ } // ----------------------------------------------------------- Constructors - + /** * Alternate constructor. @@ -137,7 +138,7 @@ * Underlying socket. */ private NioChannel socket; - + /** * Selector pool, for blocking reads and blocking writes */ @@ -159,7 +160,7 @@ // --------------------------------------------------------- Public Methods /** - * Recycle the input buffer. This should be called when closing the + * Recycle the input buffer. This should be called when closing the * connection. */ @Override @@ -178,7 +179,7 @@ /** * End processing of current HTTP request. - * Note: All bytes of the current request should have been already + * Note: All bytes of the current request should have been already * consumed. This method only resets all the pointers so that we are ready * to parse the next HTTP request. */ @@ -195,14 +196,14 @@ } /** - * Read the request line. This function is meant to be used during the - * HTTP request header parsing. Do NOT attempt to read the request body + * Read the request line. This function is meant to be used during the + * HTTP request header parsing. Do NOT attempt to read the request body * using it. * * @throws IOException If an exception occurs during the underlying socket * read operations, or if the given buffer is not big enough to accommodate * the whole line. - * @return true if data is properly fed; false if no data is available + * @return true if data is properly fed; false if no data is available * immediately and thread should be freed */ @Override @@ -217,7 +218,7 @@ if ( parsingRequestLinePhase == 0 ) { byte chr = 0; do { - + // Read new bytes if needed if (pos >= lastValid) { if (useAvailableDataOnly) { @@ -248,7 +249,7 @@ if ( parsingRequestLinePhase == 2 ) { // // Reading the method name - // Method name is always US-ASCII + // Method name is a token // boolean space = false; while (!space) { @@ -257,21 +258,20 @@ if (!fill(true, false)) //request line parsing return false; } - // Spec says no CR or LF in method name - if (buf[pos] == Constants.CR || buf[pos] == Constants.LF) { - throw new IllegalArgumentException( - sm.getString("iib.invalidmethod")); - } + // Spec says method name is a token followed by a single SP but + // also be tolerant of multiple SP and/or HT. if (buf[pos] == Constants.SP || buf[pos] == Constants.HT) { space = true; request.method().setBytes(buf, parsingRequestLineStart, pos - parsingRequestLineStart); + } else if (!HttpParser.isToken(buf[pos])) { + throw new IllegalArgumentException(sm.getString("iib.invalidmethod")); } pos++; } parsingRequestLinePhase = 3; } if ( parsingRequestLinePhase == 3 ) { - // Spec says single SP but also be tolerant of multiple and/or HT + // Spec says single SP but also be tolerant of multiple SP and/or HT boolean space = true; while (space) { // Read new bytes if needed @@ -290,7 +290,7 @@ } if (parsingRequestLinePhase == 4) { // Mark the current buffer position - + int end = 0; // // Reading the URI @@ -305,21 +305,22 @@ if (buf[pos] == Constants.SP || buf[pos] == Constants.HT) { space = true; end = pos; - } else if ((buf[pos] == Constants.CR) + } else if ((buf[pos] == Constants.CR) || (buf[pos] == Constants.LF)) { // HTTP/0.9 style request parsingRequestLineEol = true; space = true; end = pos; - } else if ((buf[pos] == Constants.QUESTION) - && (parsingRequestLineQPos == -1)) { + } else if ((buf[pos] == Constants.QUESTION) && (parsingRequestLineQPos == -1)) { parsingRequestLineQPos = pos; + } else if (HttpParser.isNotRequestTarget(buf[pos])) { + throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget")); } pos++; } request.unparsedURI().setBytes(buf, parsingRequestLineStart, end - parsingRequestLineStart); if (parsingRequestLineQPos >= 0) { - request.queryString().setBytes(buf, parsingRequestLineQPos + 1, + request.queryString().setBytes(buf, parsingRequestLineQPos + 1, end - parsingRequestLineQPos - 1); request.requestURI().setBytes(buf, parsingRequestLineStart, parsingRequestLineQPos - parsingRequestLineStart); } else { @@ -348,10 +349,10 @@ // Mark the current buffer position end = 0; } - if (parsingRequestLinePhase == 6) { + if (parsingRequestLinePhase == 6) { // // Reading the protocol - // Protocol is always US-ASCII + // Protocol is always "HTTP/" DIGIT "." DIGIT // while (!parsingRequestLineEol) { // Read new bytes if needed @@ -359,17 +360,19 @@ if (!fill(true, false)) //request line parsing return false; } - + if (buf[pos] == Constants.CR) { end = pos; } else if (buf[pos] == Constants.LF) { if (end == 0) end = pos; parsingRequestLineEol = true; + } else if (!HttpParser.isHttpProtocol(buf[pos])) { + throw new IllegalArgumentException(sm.getString("iib.invalidHttpProtocol")); } pos++; } - + if ( (end - parsingRequestLineStart) > 0) { request.protocol().setBytes(buf, parsingRequestLineStart, end - parsingRequestLineStart); } else { @@ -383,7 +386,7 @@ } throw new IllegalStateException("Invalid request line parse phase:"+parsingRequestLinePhase); } - + private void expand(int newsize) { if ( newsize > buf.length ) { if (parsingHeader) { @@ -398,7 +401,7 @@ buf = tmp; } } - + /** * Perform blocking read with a timeout if desired * @param timeout boolean - if we want to use the timeout data @@ -407,7 +410,7 @@ * @throws IOException if a socket exception occurs * @throws EOFException if end of stream is reached */ - + private int readSocket(boolean timeout, boolean block) throws IOException { int nRead = 0; socket.getBufHandler().getReadBuffer().clear(); @@ -429,7 +432,7 @@ socket.getIOChannel().socket().getSoTimeout()); } catch ( EOFException eof ) { nRead = -1; - } finally { + } finally { if ( selector != null ) pool.put(selector); } } else { @@ -462,7 +465,7 @@ } HeaderParseStatus status = HeaderParseStatus.HAVE_MORE_HEADERS; - + do { status = parseHeader(); // Checking that @@ -491,7 +494,7 @@ /** * Parse an HTTP header. - * + * * @return false after reading a blank line (which indicates that the * HTTP header parsing is done */ @@ -507,7 +510,7 @@ // Read new bytes if needed if (pos >= lastValid) { - if (!fill(true,false)) {//parse header + if (!fill(true,false)) {//parse header headerParsePos = HeaderParsePosition.HEADER_START; return HeaderParseStatus.NEED_MORE_DATA; } @@ -543,7 +546,7 @@ // Read new bytes if needed if (pos >= lastValid) { - if (!fill(true,false)) { //parse header + if (!fill(true,false)) { //parse header return HeaderParseStatus.NEED_MORE_DATA; } } @@ -558,7 +561,7 @@ headerData.realPos = pos; headerData.lastSignificantChar = pos; break; - } else if (!HTTP_TOKEN_CHAR[chr]) { + } else if (!HttpParser.isToken(chr)) { // If a non-token header is detected, skip the line and // ignore the header headerData.lastSignificantChar = pos; @@ -590,7 +593,7 @@ while (true) { // Read new bytes if needed if (pos >= lastValid) { - if (!fill(true,false)) {//parse header + if (!fill(true,false)) {//parse header //HEADER_VALUE_START return HeaderParseStatus.NEED_MORE_DATA; } @@ -613,7 +616,7 @@ // Read new bytes if needed if (pos >= lastValid) { - if (!fill(true,false)) {//parse header + if (!fill(true,false)) {//parse header //HEADER_VALUE return HeaderParseStatus.NEED_MORE_DATA; } @@ -646,7 +649,7 @@ // Read new bytes if needed if (pos >= lastValid) { if (!fill(true,false)) {//parse header - + //HEADER_MULTI_LINE return HeaderParseStatus.NEED_MORE_DATA; } @@ -672,7 +675,7 @@ headerData.recycle(); return HeaderParseStatus.HAVE_MORE_HEADERS; } - + public int getParsingRequestLinePhase() { return parsingRequestLinePhase; } @@ -771,7 +774,7 @@ /** * Fill the internal buffer using data from the underlying input stream. - * + * * @return false if at end of stream */ @Override @@ -780,7 +783,7 @@ } protected boolean fill(boolean timeout, boolean block) throws IOException, EOFException { - + boolean read = false; @@ -809,7 +812,7 @@ * This class is an input buffer which will read its data from an input * stream. */ - protected class SocketInputBuffer + protected class SocketInputBuffer implements InputBuffer { @@ -817,7 +820,7 @@ * Read bytes into the specified chunk. */ @Override - public int doRead(ByteChunk chunk, Request req ) + public int doRead(ByteChunk chunk, Request req ) throws IOException { if (pos >= lastValid) { --- java/org/apache/coyote/http11/LocalStrings.properties.orig 2017-03-28 15:06:40.632389841 -0400 +++ java/org/apache/coyote/http11/LocalStrings.properties 2017-03-28 15:06:40.649389926 -0400 @@ -42,8 +42,10 @@ iib.apr.sslGeneralError=An APR general error was returned by the SSL read operation on APR/native socket [{0}] with wrapper [{1}]. It will be treated as EAGAIN and the socket returned to the poller. iib.eof.error=Unexpected EOF read on the socket -iib.invalidheader=The HTTP header line [{0}] does not conform to RFC 2616 and has been ignored. -iib.invalidmethod=Invalid character (CR or LF) found in method name +iib.invalidheader=The HTTP header line [{0}] does not conform to RFC 7230 and has been ignored. +iib.invalidmethod=Invalid character found in method name. HTTP method names must be tokens +iib.invalidRequestTarget=Invalid character found in the request target. The valid characters are defined in RFC 7230 and RFC 3986 +iib.invalidHttpProtocol=Invalid character found in the HTTP protocol iib.parseheaders.ise.error=Unexpected state: headers already parsed. Buffer not recycled? iib.requestheadertoolarge.error=Request header is too large --- java/org/apache/tomcat/util/http/parser/HttpParser.java.orig 2017-03-28 15:06:40.633389846 -0400 +++ java/org/apache/tomcat/util/http/parser/HttpParser.java 2017-03-28 15:06:40.657389967 -0400 @@ -23,6 +23,11 @@ import java.util.Locale; import java.util.Map; +import org.apache.juli.logging.Log; +import org.apache.juli.logging.LogFactory; + +import org.apache.tomcat.util.res.StringManager; + /** * HTTP header value parser implementation. Parsing HTTP headers as per RFC2616 * is not always as simple as it first appears. For headers that only use tokens @@ -53,9 +58,19 @@ private static final Map fieldTypes = new HashMap(); - // Arrays used by isToken(), isHex() - private static final boolean isToken[] = new boolean[128]; - private static final boolean isHex[] = new boolean[128]; + private static final StringManager sm = StringManager.getManager(HttpParser.class); + + private static final Log log = LogFactory.getLog(HttpParser.class); + + private static final int ARRAY_SIZE = 128; + + private static final boolean[] IS_CONTROL = new boolean[ARRAY_SIZE]; + private static final boolean[] IS_SEPARATOR = new boolean[ARRAY_SIZE]; + private static final boolean[] IS_TOKEN = new boolean[ARRAY_SIZE]; + private static final boolean[] IS_HEX = new boolean[ARRAY_SIZE]; + private static final boolean[] IS_NOT_REQUEST_TARGET = new boolean[ARRAY_SIZE]; + private static final boolean[] IS_HTTP_PROTOCOL = new boolean[ARRAY_SIZE]; + private static final boolean[] REQUEST_TARGET_ALLOW = new boolean[ARRAY_SIZE]; static { // Digest field types. @@ -77,24 +92,57 @@ // RFC2617 says nc is 8LHEX. <">8LHEX<"> will also be accepted fieldTypes.put("nc", FIELD_TYPE_LHEX); - // Setup the flag arrays - for (int i = 0; i < 128; i++) { - if (i <= 32) { // includes '\t' and ' ' - isToken[i] = false; - } else if (i == '(' || i == ')' || i == '<' || i == '>' || i == '@' || - i == ',' || i == ';' || i == ':' || i == '\\' || i == '\"' || - i == '/' || i == '[' || i == ']' || i == '?' || i == '=' || - i == '{' || i == '}') { - isToken[i] = false; - } else { - isToken[i] = true; + String prop = System.getProperty("tomcat.util.http.parser.HttpParser.requestTargetAllow"); + if (prop != null) { + for (int i = 0; i < prop.length(); i++) { + char c = prop.charAt(i); + if (c == '{' || c == '}' || c == '|') { + REQUEST_TARGET_ALLOW[c] = true; + } else { + log.warn(sm.getString("httpparser.invalidRequestTargetCharacter", c)); + } } + } - if (i >= '0' && i <= '9' || i >= 'A' && i <= 'F' || - i >= 'a' && i <= 'f') { - isHex[i] = true; - } else { - isHex[i] = false; + for (int i = 0; i < ARRAY_SIZE; i++) { + // Control> 0-31, 127 + if (i < 32 || i == 127) { + IS_CONTROL[i] = true; + } + + // Separator + if ( i == '(' || i == ')' || i == '<' || i == '>' || i == '@' || + i == ',' || i == ';' || i == ':' || i == '\\' || i == '\"' || + i == '/' || i == '[' || i == ']' || i == '?' || i == '=' || + i == '{' || i == '}' || i == ' ' || i == '\t') { + IS_SEPARATOR[i] = true; + } + + // Token: Anything 0-127 that is not a control and not a separator + if (!IS_CONTROL[i] && !IS_SEPARATOR[i] && i < 128) { + IS_TOKEN[i] = true; + } + + // Hex: 0-9, a-f, A-F + if ((i >= '0' && i <='9') || (i >= 'a' && i <= 'f') || (i >= 'A' && i <= 'F')) { + IS_HEX[i] = true; + } + + // Not valid for request target. + // Combination of multiple rules from RFC7230 and RFC 3986. Must be + // ASCII, no controls plus a few additional characters excluded + if (IS_CONTROL[i] || i > 127 || + i == ' ' || i == '\"' || i == '#' || i == '<' || i == '>' || i == '\\' || + i == '^' || i == '`' || i == '{' || i == '|' || i == '}') { + if (!REQUEST_TARGET_ALLOW[i]) { + IS_NOT_REQUEST_TARGET[i] = true; + } + } + + // Not valid for HTTP protocol + // "HTTP/" DIGIT "." DIGIT + if (i == 'H' || i == 'T' || i == 'P' || i == '/' || i == '.' || (i >= '0' && i <= '9')) { + IS_HTTP_PROTOCOL[i] = true; } } } @@ -228,6 +276,7 @@ return new MediaType(type, subtype, parameters); } + public static String unquote(String input) { if (input == null || input.length() < 2) { return input; @@ -258,24 +307,49 @@ return result.toString(); } - private static boolean isToken(int c) { + + public static boolean isToken(int c) { // Fast for correct values, slower for incorrect ones try { - return isToken[c]; + return IS_TOKEN[c]; } catch (ArrayIndexOutOfBoundsException ex) { return false; } } - private static boolean isHex(int c) { - // Fast for correct values, slower for incorrect ones + + public static boolean isHex(int c) { + // Fast for correct values, slower for some incorrect ones + try { + return IS_HEX[c]; + } catch (ArrayIndexOutOfBoundsException ex) { + return false; + } + } + + + public static boolean isNotRequestTarget(int c) { + // Fast for valid request target characters, slower for some incorrect + // ones try { - return isHex[c]; + return IS_NOT_REQUEST_TARGET[c]; + } catch (ArrayIndexOutOfBoundsException ex) { + return true; + } + } + + + public static boolean isHttpProtocol(int c) { + // Fast for valid HTTP protocol characters, slower for some incorrect + // ones + try { + return IS_HTTP_PROTOCOL[c]; } catch (ArrayIndexOutOfBoundsException ex) { return false; } } + // Skip any LWS and return the next char private static int skipLws(StringReader input, boolean withReset) throws IOException { --- webapps/docs/changelog.xml.orig 2017-03-28 15:06:40.634389851 -0400 +++ webapps/docs/changelog.xml 2017-03-28 15:11:46.543926068 -0400 @@ -64,6 +64,24 @@ 60409: When unable to complete sendfile request, ensure the Processor will be added to the cache only once. (markt/violetagg) + + Ensure that requests with HTTP method names that are not tokens (as + required by RFC 7231) are rejected with a 400 response. (markt) + + + Correct the HTTP header parser so that DEL is not treated as a valid + token character. (markt) + + + Add additional checks for valid characters to the HTTP request line + parsing so invalid request lines are rejected sooner. (markt) + + + 60594: Allow some invalid characters that were recently + restricted to be processed in requests by using the system property + tomcat.util.http.parser.HttpParser.requestTargetAllow. + (csutherl) + --- webapps/docs/config/systemprops.xml.orig 2017-03-28 15:06:40.635389856 -0400 +++ webapps/docs/config/systemprops.xml 2017-03-28 15:06:40.657389967 -0400 @@ -708,6 +708,16 @@

If not specified, the default value of 3 will be used.

+ +

A string comprised of characters the server should allow even when they are not encoded. + These characters would normally result in a 400 status.

+

The acceptable characters for this property are: |, { + , and }

+

WARNING: Use of this option will expose the server to CVE-2016-6816. +

+

If not specified, the default value of null will be used.

+ + --- java/org/apache/tomcat/util/http/parser/LocalStrings.properties.orig 2017-03-28 15:06:40.637389866 -0400 +++ java/org/apache/tomcat/util/http/parser/LocalStrings.properties 2017-03-28 15:06:40.657389967 -0400 @@ -0,0 +1,16 @@ +# 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. + +httpparser.invalidRequestTargetCharacter=Character [{0}] is not allowed and will continue to be rejected. --- test/org/apache/tomcat/util/http/parser/TestHttpParser.java.orig 2017-03-28 15:06:40.638389871 -0400 +++ test/org/apache/tomcat/util/http/parser/TestHttpParser.java 2017-03-28 15:06:40.646389911 -0400 @@ -0,0 +1,28 @@ +/* + * 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.tomcat.util.http.parser; + +import org.junit.Assert; +import org.junit.Test; + +public class TestHttpParser { + + @Test + public void testTokenDel() { + Assert.assertFalse("DEL is not a token", HttpParser.isToken(127)); + } +} --- test/org/apache/coyote/http11/TestInternalInputBuffer.java.orig 2017-03-28 15:06:40.639389876 -0400 +++ test/org/apache/coyote/http11/TestInternalInputBuffer.java 2017-03-28 15:06:40.642389891 -0400 @@ -478,4 +478,61 @@ } } + + + @Test + public void testInvalidMethod() { + + InvalidMethodClient client = new InvalidMethodClient(); + + client.doRequest(); + assertTrue(client.getResponseLine(), client.isResponse400()); + assertTrue(client.isResponseBodyOK()); + } + + + /** + * Bug 48839 test client. + */ + private class InvalidMethodClient extends SimpleHttpClient { + + private Exception doRequest() { + + Tomcat tomcat = getTomcatInstance(); + + tomcat.addContext("", TEMP_DIR); + + try { + tomcat.start(); + setPort(tomcat.getConnector().getLocalPort()); + + // Open connection + connect(); + + String[] request = new String[1]; + request[0] = + "GET" + (char) 0 + " /test HTTP/1.1" + CRLF + + "Host: localhost:8080" + CRLF + + "Connection: close" + CRLF + + CRLF; + + setRequest(request); + processRequest(); // blocks until response has been read + + // Close the connection + disconnect(); + } catch (Exception e) { + return e; + } + return null; + } + + @Override + public boolean isResponseBodyOK() { + if (getResponseBody() == null) { + return false; + } + return true; + } + } }