--- 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<S> 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<String,Integer> fieldTypes =
new HashMap<String,Integer>();
- // 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 @@
<bug>60409</bug>: When unable to complete sendfile request, ensure the
Processor will be added to the cache only once. (markt/violetagg)
</fix>
+ <fix>
+ Ensure that requests with HTTP method names that are not tokens (as
+ required by RFC 7231) are rejected with a 400 response. (markt)
+ </fix>
+ <fix>
+ Correct the HTTP header parser so that DEL is not treated as a valid
+ token character. (markt)
+ </fix>
+ <add>
+ Add additional checks for valid characters to the HTTP request line
+ parsing so invalid request lines are rejected sooner. (markt)
+ </add>
+ <add>
+ <bug>60594</bug>: Allow some invalid characters that were recently
+ restricted to be processed in requests by using the system property
+ <code>tomcat.util.http.parser.HttpParser.requestTargetAllow</code>.
+ (csutherl)
+ </add>
</changelog>
</subsection>
</section>
--- 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 @@
<p>If not specified, the default value of <code>3</code> will be used.</p>
</property>
+ <property name="tomcat.util.http.parser.HttpParser.requestTargetAllow">
+ <p>A string comprised of characters the server should allow even when they are not encoded.
+ These characters would normally result in a 400 status.</p>
+ <p>The acceptable characters for this property are: <code>|</code>, <code>{</code>
+ , and <code>}</code></p>
+ <p><strong>WARNING</strong>: Use of this option will expose the server to CVE-2016-6816.
+ </p>
+ <p>If not specified, the default value of <code>null</code> will be used.</p>
+ </property>
+
</properties>
</section>
--- 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;
+ }
+ }
}