Skip to content

Commit

Permalink
UNDERTOW-881 AJP and HTTP/2 listeners ignore max header and parameter…
Browse files Browse the repository at this point in the history
… limits
  • Loading branch information
stuartwdouglas committed Oct 31, 2016
1 parent 4bd0ee8 commit ce5ffeb
Show file tree
Hide file tree
Showing 18 changed files with 123 additions and 64 deletions.
2 changes: 1 addition & 1 deletion core/src/main/java/io/undertow/UndertowMessages.java
Expand Up @@ -150,7 +150,7 @@ public interface UndertowMessages {
RuntimeException tooManyQueryParameters(int noParams); RuntimeException tooManyQueryParameters(int noParams);


@Message(id = 40, value = "To many headers, cannot have more than %s header") @Message(id = 40, value = "To many headers, cannot have more than %s header")
RuntimeException tooManyHeaders(int noParams); String tooManyHeaders(int noParams);


@Message(id = 41, value = "Channel is closed") @Message(id = 41, value = "Channel is closed")
ClosedChannelException channelIsClosed(); ClosedChannelException channelIsClosed();
Expand Down
4 changes: 4 additions & 0 deletions core/src/main/java/io/undertow/UndertowOptions.java
Expand Up @@ -77,6 +77,8 @@ public class UndertowOptions {
*/ */
public static final Option<Integer> NO_REQUEST_TIMEOUT = Option.simple(UndertowOptions.class, "NO_REQUEST_TIMEOUT", Integer.class); public static final Option<Integer> NO_REQUEST_TIMEOUT = Option.simple(UndertowOptions.class, "NO_REQUEST_TIMEOUT", Integer.class);


public static final int DEFAULT_MAX_PARAMETERS = 1000;

/** /**
* The maximum number of parameters that will be parsed. This is used to protect against hash vulnerabilities. * The maximum number of parameters that will be parsed. This is used to protect against hash vulnerabilities.
* <p> * <p>
Expand All @@ -87,6 +89,8 @@ public class UndertowOptions {
*/ */
public static final Option<Integer> MAX_PARAMETERS = Option.simple(UndertowOptions.class, "MAX_PARAMETERS", Integer.class); public static final Option<Integer> MAX_PARAMETERS = Option.simple(UndertowOptions.class, "MAX_PARAMETERS", Integer.class);


public static final int DEFAULT_MAX_HEADERS = 200;

/** /**
* The maximum number of headers that will be parsed. This is used to protect against hash vulnerabilities. * The maximum number of headers that will be parsed. This is used to protect against hash vulnerabilities.
* <p> * <p>
Expand Down
Expand Up @@ -50,7 +50,7 @@ public OpenSSLALPNMethods run() {
UndertowLogger.ROOT_LOGGER.debug("OpenSSL ALPN Enabled"); UndertowLogger.ROOT_LOGGER.debug("OpenSSL ALPN Enabled");
return new OpenSSLALPNMethods(setApplicationProtocols, getApplicationProtocol); return new OpenSSLALPNMethods(setApplicationProtocols, getApplicationProtocol);
} catch (Throwable e) { } catch (Throwable e) {
UndertowLogger.ROOT_LOGGER.debug("OpenSSL ALPN Enabled", e); UndertowLogger.ROOT_LOGGER.debug("OpenSSL ALPN disabled", e);
return null; return null;
} }
} }
Expand Down
Expand Up @@ -137,6 +137,7 @@ public class Http2Channel extends AbstractFramedChannel<Http2Channel, AbstractHt
private int sendMaxFrameSize = DEFAULT_MAX_FRAME_SIZE; private int sendMaxFrameSize = DEFAULT_MAX_FRAME_SIZE;
private int receiveMaxFrameSize = DEFAULT_MAX_FRAME_SIZE; private int receiveMaxFrameSize = DEFAULT_MAX_FRAME_SIZE;
private int unackedReceiveMaxFrameSize = DEFAULT_MAX_FRAME_SIZE; //the old max frame size, this gets updated when our setting frame is acked private int unackedReceiveMaxFrameSize = DEFAULT_MAX_FRAME_SIZE; //the old max frame size, this gets updated when our setting frame is acked
private final int maxHeaders;


/** /**
* How much data we have told the remote endpoint we are prepared to accept. * How much data we have told the remote endpoint we are prepared to accept.
Expand Down Expand Up @@ -189,6 +190,7 @@ public Http2Channel(StreamConnection connectedStreamChannel, String protocol, By
this.initialReceiveWindowSize = settings.get(UndertowOptions.HTTP2_SETTINGS_INITIAL_WINDOW_SIZE, DEFAULT_INITIAL_WINDOW_SIZE); this.initialReceiveWindowSize = settings.get(UndertowOptions.HTTP2_SETTINGS_INITIAL_WINDOW_SIZE, DEFAULT_INITIAL_WINDOW_SIZE);


this.protocol = protocol == null ? Http2OpenListener.HTTP2 : protocol; this.protocol = protocol == null ? Http2OpenListener.HTTP2 : protocol;
this.maxHeaders = settings.get(UndertowOptions.MAX_HEADERS, clientSide ? -1 : UndertowOptions.DEFAULT_MAX_HEADERS);


encoderHeaderTableSize = settings.get(UndertowOptions.HTTP2_SETTINGS_HEADER_TABLE_SIZE, Hpack.DEFAULT_TABLE_SIZE); encoderHeaderTableSize = settings.get(UndertowOptions.HTTP2_SETTINGS_HEADER_TABLE_SIZE, Hpack.DEFAULT_TABLE_SIZE);
receiveMaxFrameSize = settings.get(UndertowOptions.HTTP2_SETTINGS_MAX_FRAME_SIZE, DEFAULT_MAX_FRAME_SIZE); receiveMaxFrameSize = settings.get(UndertowOptions.HTTP2_SETTINGS_MAX_FRAME_SIZE, DEFAULT_MAX_FRAME_SIZE);
Expand Down Expand Up @@ -837,6 +839,10 @@ HpackDecoder getDecoder() {
return decoder; return decoder;
} }


int getMaxHeaders() {
return maxHeaders;
}

int getPaddingBytes() { int getPaddingBytes() {
if(paddingRandom == null) { if(paddingRandom == null) {
return 0; return 0;
Expand Down
Expand Up @@ -85,7 +85,7 @@ public boolean handle(final ByteBuffer byteBuffer) throws IOException {
if (streamId == 0) { if (streamId == 0) {
throw new ConnectionErrorException(Http2Channel.ERROR_PROTOCOL_ERROR, UndertowMessages.MESSAGES.streamIdMustNotBeZeroForFrameType(Http2Channel.FRAME_TYPE_HEADERS)); throw new ConnectionErrorException(Http2Channel.ERROR_PROTOCOL_ERROR, UndertowMessages.MESSAGES.streamIdMustNotBeZeroForFrameType(Http2Channel.FRAME_TYPE_HEADERS));
} }
parser = new Http2HeadersParser(length, http2Channel.getDecoder(), http2Channel.isClient(), streamId); parser = new Http2HeadersParser(length, http2Channel.getDecoder(), http2Channel.isClient(), http2Channel.getMaxHeaders(), streamId);
if(allAreClear(flags, Http2Channel.HEADERS_FLAG_END_HEADERS)) { if(allAreClear(flags, Http2Channel.HEADERS_FLAG_END_HEADERS)) {
continuationParser = (Http2HeadersParser) parser; continuationParser = (Http2HeadersParser) parser;
} }
Expand All @@ -112,7 +112,7 @@ public boolean handle(final ByteBuffer byteBuffer) throws IOException {
break; break;
} }
case FRAME_TYPE_PUSH_PROMISE: { case FRAME_TYPE_PUSH_PROMISE: {
parser = new Http2PushPromiseParser(length, http2Channel.getDecoder(), http2Channel.isClient(), streamId); parser = new Http2PushPromiseParser(length, http2Channel.getDecoder(), http2Channel.isClient(), http2Channel.getMaxHeaders(), streamId);
if(allAreClear(flags, Http2Channel.HEADERS_FLAG_END_HEADERS)) { if(allAreClear(flags, Http2Channel.HEADERS_FLAG_END_HEADERS)) {
continuationParser = (Http2HeadersParser) parser; continuationParser = (Http2HeadersParser) parser;
} }
Expand Down
Expand Up @@ -47,6 +47,7 @@ abstract class Http2HeaderBlockParser extends Http2PushBackParser implements Hpa
private boolean invalid = false; private boolean invalid = false;
private boolean processingPseudoHeaders = true; private boolean processingPseudoHeaders = true;
private final boolean client; private final boolean client;
private final int maxHeaders;


private int currentPadding; private int currentPadding;
private final int streamId; private final int streamId;
Expand All @@ -63,15 +64,19 @@ abstract class Http2HeaderBlockParser extends Http2PushBackParser implements Hpa
SERVER_HEADERS = Collections.unmodifiableSet(server); SERVER_HEADERS = Collections.unmodifiableSet(server);
} }


Http2HeaderBlockParser(int frameLength, HpackDecoder decoder, boolean client, int streamId) { Http2HeaderBlockParser(int frameLength, HpackDecoder decoder, boolean client, int maxHeaders, int streamId) {
super(frameLength); super(frameLength);
this.decoder = decoder; this.decoder = decoder;
this.client = client; this.client = client;
this.maxHeaders = maxHeaders;
this.streamId = streamId; this.streamId = streamId;
} }


@Override @Override
protected void handleData(ByteBuffer resource, Http2FrameHeaderParser header) throws IOException { protected void handleData(ByteBuffer resource, Http2FrameHeaderParser header) throws IOException {
if(maxHeaders > 0 && headerMap.size() >= maxHeaders) {
throw new IOException(UndertowMessages.MESSAGES.tooManyHeaders(maxHeaders));
}
boolean continuationFramesComing = Bits.anyAreClear(header.flags, Http2Channel.HEADERS_FLAG_END_HEADERS); boolean continuationFramesComing = Bits.anyAreClear(header.flags, Http2Channel.HEADERS_FLAG_END_HEADERS);
if (frameRemaining == -1) { if (frameRemaining == -1) {
frameRemaining = header.length; frameRemaining = header.length;
Expand Down
Expand Up @@ -35,8 +35,8 @@ class Http2HeadersParser extends Http2HeaderBlockParser {
private boolean headersEndStream = false; private boolean headersEndStream = false;
private boolean exclusive; private boolean exclusive;


Http2HeadersParser(int frameLength, HpackDecoder hpackDecoder, boolean client, int streamId) { Http2HeadersParser(int frameLength, HpackDecoder hpackDecoder, boolean client,int maxHeaders, int streamId) {
super(frameLength, hpackDecoder, client, streamId); super(frameLength, hpackDecoder, client, maxHeaders, streamId);
} }


@Override @Override
Expand Down
Expand Up @@ -33,8 +33,8 @@ class Http2PushPromiseParser extends Http2HeaderBlockParser {
private int promisedStreamId; private int promisedStreamId;
private static final int STREAM_MASK = ~(1 << 7); private static final int STREAM_MASK = ~(1 << 7);


Http2PushPromiseParser(int frameLength, HpackDecoder hpackDecoder, boolean client, int streamId) { Http2PushPromiseParser(int frameLength, HpackDecoder hpackDecoder, boolean client, int maxHeaders, int streamId) {
super(frameLength, hpackDecoder, client, streamId); super(frameLength, hpackDecoder, client, maxHeaders, streamId);
} }


@Override @Override
Expand Down
21 changes: 16 additions & 5 deletions core/src/main/java/io/undertow/server/Connectors.java
Expand Up @@ -19,6 +19,7 @@
package io.undertow.server; package io.undertow.server;


import io.undertow.UndertowLogger; import io.undertow.UndertowLogger;
import io.undertow.UndertowOptions;
import io.undertow.server.handlers.Cookie; import io.undertow.server.handlers.Cookie;
import io.undertow.util.DateUtils; import io.undertow.util.DateUtils;
import io.undertow.util.Headers; import io.undertow.util.Headers;
Expand Down Expand Up @@ -236,15 +237,25 @@ public static void executeRootHandler(final HttpHandler handler, final HttpServe
} }
} }



/** /**
* Sets the request path and query parameters, decoding to the requested charset. * Sets the request path and query parameters, decoding to the requested charset.
* *
* @param exchange The exchange * @param exchange The exchange
* @param encodedPath The encoded path * @param encodedPath The encoded path
* @param charset The charset * @param charset The charset
*/ */
@Deprecated
public static void setExchangeRequestPath(final HttpServerExchange exchange, final String encodedPath, final String charset, boolean decode, final boolean allowEncodedSlash, StringBuilder decodeBuffer) { public static void setExchangeRequestPath(final HttpServerExchange exchange, final String encodedPath, final String charset, boolean decode, final boolean allowEncodedSlash, StringBuilder decodeBuffer) {
setExchangeRequestPath(exchange, encodedPath, charset, decode, allowEncodedSlash, decodeBuffer, exchange.getConnection().getUndertowOptions().get(UndertowOptions.MAX_PARAMETERS, UndertowOptions.DEFAULT_MAX_PARAMETERS));
}
/**
* Sets the request path and query parameters, decoding to the requested charset.
*
* @param exchange The exchange
* @param encodedPath The encoded path
* @param charset The charset
*/
public static void setExchangeRequestPath(final HttpServerExchange exchange, final String encodedPath, final String charset, boolean decode, final boolean allowEncodedSlash, StringBuilder decodeBuffer, int maxParameters) {
boolean requiresDecode = false; boolean requiresDecode = false;
for (int i = 0; i < encodedPath.length(); ++i) { for (int i = 0; i < encodedPath.length(); ++i) {
char c = encodedPath.charAt(i); char c = encodedPath.charAt(i);
Expand All @@ -261,7 +272,7 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin
exchange.setRequestURI(encodedPart); exchange.setRequestURI(encodedPart);
final String qs = encodedPath.substring(i + 1); final String qs = encodedPath.substring(i + 1);
exchange.setQueryString(qs); exchange.setQueryString(qs);
URLUtils.parseQueryString(qs, exchange, charset, decode); URLUtils.parseQueryString(qs, exchange, charset, decode, maxParameters);
return; return;
} else if(c == ';') { } else if(c == ';') {
String part; String part;
Expand All @@ -277,15 +288,15 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin
if (encodedPath.charAt(j) == '?') { if (encodedPath.charAt(j) == '?') {
exchange.setRequestURI(encodedPath.substring(0, j)); exchange.setRequestURI(encodedPath.substring(0, j));
String pathParams = encodedPath.substring(i + 1, j); String pathParams = encodedPath.substring(i + 1, j);
URLUtils.parsePathParms(pathParams, exchange, charset, decode); URLUtils.parsePathParms(pathParams, exchange, charset, decode, maxParameters);
String qs = encodedPath.substring(j + 1); String qs = encodedPath.substring(j + 1);
exchange.setQueryString(qs); exchange.setQueryString(qs);
URLUtils.parseQueryString(qs, exchange, charset, decode); URLUtils.parseQueryString(qs, exchange, charset, decode, maxParameters);
return; return;
} }
} }
exchange.setRequestURI(encodedPath); exchange.setRequestURI(encodedPath);
URLUtils.parsePathParms(encodedPath.substring(i + 1), exchange, charset, decode); URLUtils.parsePathParms(encodedPath.substring(i + 1), exchange, charset, decode, maxParameters);
return; return;
} else if(c == '%' || c == '+') { } else if(c == '%' || c == '+') {
requiresDecode = true; requiresDecode = true;
Expand Down
Expand Up @@ -90,7 +90,7 @@ public AjpOpenListener(final ByteBufferPool pool, final OptionMap undertowOption
PooledByteBuffer buf = pool.allocate(); PooledByteBuffer buf = pool.allocate();
this.bufferSize = buf.getBuffer().remaining(); this.bufferSize = buf.getBuffer().remaining();
buf.close(); buf.close();
parser = new AjpRequestParser(undertowOptions.get(URL_CHARSET, StandardCharsets.UTF_8.name()), undertowOptions.get(DECODE_URL, true)); parser = new AjpRequestParser(undertowOptions.get(URL_CHARSET, StandardCharsets.UTF_8.name()), undertowOptions.get(DECODE_URL, true), undertowOptions.get(UndertowOptions.MAX_PARAMETERS, UndertowOptions.DEFAULT_MAX_PARAMETERS), undertowOptions.get(UndertowOptions.MAX_HEADERS, UndertowOptions.DEFAULT_MAX_HEADERS));
connectorStatistics = new ConnectorStatisticsImpl(); connectorStatistics = new ConnectorStatisticsImpl();
statisticsEnabled = undertowOptions.get(UndertowOptions.ENABLE_CONNECTOR_STATISTICS, false); statisticsEnabled = undertowOptions.get(UndertowOptions.ENABLE_CONNECTOR_STATISTICS, false);
} }
Expand Down Expand Up @@ -165,7 +165,7 @@ public void setUndertowOptions(final OptionMap undertowOptions) {
} }
this.undertowOptions = undertowOptions; this.undertowOptions = undertowOptions;
statisticsEnabled = undertowOptions.get(UndertowOptions.ENABLE_CONNECTOR_STATISTICS, false); statisticsEnabled = undertowOptions.get(UndertowOptions.ENABLE_CONNECTOR_STATISTICS, false);
parser = new AjpRequestParser(undertowOptions.get(URL_CHARSET, StandardCharsets.UTF_8.name()), undertowOptions.get(DECODE_URL, true)); parser = new AjpRequestParser(undertowOptions.get(URL_CHARSET, StandardCharsets.UTF_8.name()), undertowOptions.get(DECODE_URL, true), undertowOptions.get(UndertowOptions.MAX_PARAMETERS, UndertowOptions.DEFAULT_MAX_PARAMETERS), undertowOptions.get(UndertowOptions.MAX_HEADERS, UndertowOptions.DEFAULT_MAX_HEADERS));
} }


@Override @Override
Expand Down
Expand Up @@ -67,6 +67,8 @@ public class AjpRequestParser {


private final String encoding; private final String encoding;
private final boolean doDecode; private final boolean doDecode;
private final int maxParameters;
private final int maxHeaders;


private static final HttpString[] HTTP_HEADERS; private static final HttpString[] HTTP_HEADERS;


Expand Down Expand Up @@ -169,13 +171,15 @@ public class AjpRequestParser {
ATTRIBUTES[13] = STORED_METHOD; ATTRIBUTES[13] = STORED_METHOD;
} }


public AjpRequestParser(String encoding, boolean doDecode) { public AjpRequestParser(String encoding, boolean doDecode, int maxParameters, int maxHeaders) {
this.encoding = encoding; this.encoding = encoding;
this.doDecode = doDecode; this.doDecode = doDecode;
this.maxParameters = maxParameters;
this.maxHeaders = maxHeaders;
} }




public void parse(final ByteBuffer buf, final AjpRequestParseState state, final HttpServerExchange exchange) throws IOException { public void parse(final ByteBuffer buf, final AjpRequestParseState state, final HttpServerExchange exchange) throws IOException, BadRequestException {
if (!buf.hasRemaining()) { if (!buf.hasRemaining()) {
return; return;
} }
Expand Down Expand Up @@ -250,7 +254,7 @@ public void parse(final ByteBuffer buf, final AjpRequestParseState state, final
exchange.setRequestURI(result.value); exchange.setRequestURI(result.value);
exchange.setRequestPath(res); exchange.setRequestPath(res);
exchange.setRelativePath(res); exchange.setRelativePath(res);
URLUtils.parsePathParms(result.value.substring(colon + 1), exchange, encoding, doDecode && result.containsUrlCharacters); URLUtils.parsePathParms(result.value.substring(colon + 1), exchange, encoding, doDecode && result.containsUrlCharacters, maxParameters);
} }
} else { } else {
state.state = AjpRequestParseState.READING_REQUEST_URI; state.state = AjpRequestParseState.READING_REQUEST_URI;
Expand Down Expand Up @@ -313,6 +317,9 @@ public void parse(final ByteBuffer buf, final AjpRequestParseState state, final
return; return;
} else { } else {
state.numHeaders = result.value; state.numHeaders = result.value;
if(state.numHeaders > maxHeaders) {
throw new BadRequestException(UndertowMessages.MESSAGES.tooManyHeaders(state.numHeaders));
}
} }
} }
case AjpRequestParseState.READING_HEADERS: { case AjpRequestParseState.READING_HEADERS: {
Expand Down Expand Up @@ -394,7 +401,7 @@ public void parse(final ByteBuffer buf, final AjpRequestParseState state, final
if (state.currentAttribute.equals(QUERY_STRING)) { if (state.currentAttribute.equals(QUERY_STRING)) {
String resultAsQueryString = result == null ? "" : result; String resultAsQueryString = result == null ? "" : result;
exchange.setQueryString(resultAsQueryString); exchange.setQueryString(resultAsQueryString);
URLUtils.parseQueryString(resultAsQueryString, exchange, encoding, doDecode); URLUtils.parseQueryString(resultAsQueryString, exchange, encoding, doDecode, maxParameters);
} else if (state.currentAttribute.equals(REMOTE_USER)) { } else if (state.currentAttribute.equals(REMOTE_USER)) {
exchange.putAttachment(ExternalAuthenticationMechanism.EXTERNAL_PRINCIPAL, result); exchange.putAttachment(ExternalAuthenticationMechanism.EXTERNAL_PRINCIPAL, result);
} else if (state.currentAttribute.equals(AUTH_TYPE)) { } else if (state.currentAttribute.equals(AUTH_TYPE)) {
Expand Down Expand Up @@ -555,6 +562,11 @@ enum StringType {
URL, URL,
QUERY_STRING, QUERY_STRING,
OTHER OTHER
}


public static class BadRequestException extends Exception {
public BadRequestException(String msg) {
super(msg);
}
} }
} }

0 comments on commit ce5ffeb

Please sign in to comment.