Skip to content

Commit

Permalink
Add additional checks for valid characters to the HTTP request line
Browse files Browse the repository at this point in the history 
parsing so invalid request lines are rejected sooner.

git-svn-id: https://svn.apache.org/repos/asf/tomcat/tc7.0.x/trunk@1767675 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
@markt-asf
markt-asf committed Nov 2, 2016
1 parent be6b5a8 commit cdc0a93
Show file tree
Hide file tree
Showing 7 changed files with 141 additions and 134 deletions.
54 changes: 1 addition & 53 deletions java/org/apache/coyote/http11/AbstractInputBuffer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);


/**
Expand Down
54 changes: 29 additions & 25 deletions java/org/apache/coyote/http11/InternalAprInputBuffer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -70,7 +71,7 @@ public InternalAprInputBuffer(Request request, int headerBufferSize) {

parsingHeader = true;
swallowInput = true;

}


Expand All @@ -93,7 +94,7 @@ public InternalAprInputBuffer(Request request, int headerBufferSize) {
// --------------------------------------------------------- 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
Expand All @@ -105,14 +106,14 @@ public void recycle() {


/**
* 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
Expand Down Expand Up @@ -177,7 +178,7 @@ public boolean parseRequestLine(boolean useAvailableData)
if (buf[pos] == Constants.SP || buf[pos] == Constants.HT) {
space = true;
request.method().setBytes(buf, start, pos - start);
} else if (!HTTP_TOKEN_CHAR[buf[pos]]) {
} else if (!HttpParser.isToken(buf[pos])) {
throw new IllegalArgumentException(sm.getString("iib.invalidmethod"));
}

Expand Down Expand Up @@ -222,15 +223,16 @@ public boolean parseRequestLine(boolean useAvailableData)
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++;
Expand All @@ -239,7 +241,7 @@ public boolean parseRequestLine(boolean useAvailableData)

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 {
Expand Down Expand Up @@ -267,7 +269,7 @@ public boolean parseRequestLine(boolean useAvailableData)

//
// Reading the protocol
// Protocol is always US-ASCII
// Protocol is always "HTTP/" DIGIT "." DIGIT
//

while (!eol) {
Expand All @@ -284,6 +286,8 @@ public boolean parseRequestLine(boolean useAvailableData)
if (end == 0)
end = pos;
eol = true;
} else if (!HttpParser.isHttpProtocol(buf[pos])) {
throw new IllegalArgumentException(sm.getString("iib.invalidHttpProtocol"));
}

pos++;
Expand All @@ -295,7 +299,7 @@ public boolean parseRequestLine(boolean useAvailableData)
} else {
request.protocol().setString("");
}

return true;

}
Expand Down Expand Up @@ -324,7 +328,7 @@ public boolean parseHeaders()

/**
* Parse an HTTP header.
*
*
* @return false after reading a blank line (which indicates that the
* HTTP header parsing is done
*/
Expand Down Expand Up @@ -382,7 +386,7 @@ private boolean parseHeader()
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);
Expand Down Expand Up @@ -488,14 +492,14 @@ private boolean parseHeader()

}


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
Expand All @@ -519,16 +523,16 @@ private void skipLine(int start) throws IOException {
lastRealByte - start + 1, Charset.forName("ISO-8859-1"))));
}
}


// ---------------------------------------------------- InputBuffer Methods


/**
* Read some bytes.
*/
@Override
public int doRead(ByteChunk chunk, Request req)
public int doRead(ByteChunk chunk, Request req)
throws IOException {

if (lastActiveFilter == -1)
Expand Down Expand Up @@ -556,11 +560,11 @@ protected boolean fill(boolean block) throws IOException {
// 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()
Expand Down Expand Up @@ -592,7 +596,7 @@ protected boolean fill()
} 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];
Expand Down Expand Up @@ -638,15 +642,15 @@ protected boolean fill()
* This class is an input buffer which will read its data from an input
* stream.
*/
protected class SocketInputBuffer
protected class SocketInputBuffer
implements InputBuffer {


/**
* 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) {
Expand Down
Loading

0 comments on commit cdc0a93

Please sign in to comment.