Skip to content

Commit

Permalink
Merge pull request #1438 from baranowb/UNDERTOW-2200_options
Browse files Browse the repository at this point in the history
[UNDERTOW-2200] - Path and query parameters are not decoded properly …
  • Loading branch information
fl4via committed Feb 6, 2023
2 parents 9230a27 + e2a1f3a commit 6da238e
Show file tree
Hide file tree
Showing 11 changed files with 131 additions and 30 deletions.
16 changes: 15 additions & 1 deletion core/src/main/java/io/undertow/UndertowOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,25 @@ public class UndertowOptions {
* this is disabled by default.
* <p>
* Defaults to false
*
* <p>
* See <a href="http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2007-0450">CVE-2007-0450</a>
* @deprecated - this option was interpreted improperly.
* @See {@link #DECODE_SLASH}
*/
public static final Option<Boolean> ALLOW_ENCODED_SLASH = Option.simple(UndertowOptions.class, "ALLOW_ENCODED_SLASH", Boolean.class);

/**
* If a request comes in with encoded / characters (i.e. %2F), will these be decoded.
* <p>
* This can cause security problems if a front end proxy does not perform the same decoding, and as a result
* this is disabled by default.
* <p>
* If this option is set explicitly, the {@link #ALLOW_ENCODED_SLASH} is ignored. Should default to <b>true</b>
* <p>
* See <a href="http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2007-0450">CVE-2007-0450</a>
*/
public static final Option<Boolean> DECODE_SLASH = Option.simple(UndertowOptions.class, "DECODE_SLASH", Boolean.class);

/**
* If this is true then the parser will decode the URL and query parameters using the selected character encoding (UTF-8 by default). If this is false they will
* not be decoded. This will allow a later handler to decode them into whatever charset is desired.
Expand Down
14 changes: 8 additions & 6 deletions core/src/main/java/io/undertow/server/Connectors.java
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,8 @@ public static void executeRootHandler(final HttpHandler handler, final HttpServe
@Deprecated
public static void setExchangeRequestPath(final HttpServerExchange exchange, final String encodedPath, final String charset, boolean decode, final boolean allowEncodedSlash, StringBuilder decodeBuffer) {
try {
setExchangeRequestPath(exchange, encodedPath, charset, decode, allowEncodedSlash, decodeBuffer, exchange.getConnection().getUndertowOptions().get(UndertowOptions.MAX_PARAMETERS, UndertowOptions.DEFAULT_MAX_PARAMETERS));
final boolean slashDecodingFlag = URLUtils.getSlashDecodingFlag(allowEncodedSlash, exchange.getConnection().getUndertowOptions().get(UndertowOptions.DECODE_SLASH));
setExchangeRequestPath(exchange, encodedPath, charset, decode, slashDecodingFlag, decodeBuffer, exchange.getConnection().getUndertowOptions().get(UndertowOptions.MAX_PARAMETERS, UndertowOptions.DEFAULT_MAX_PARAMETERS));
} catch (ParameterLimitException e) {
throw new RuntimeException(e);
}
Expand All @@ -461,10 +462,11 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin
*/
public static void setExchangeRequestPath(final HttpServerExchange exchange, final String encodedPath, StringBuilder decodeBuffer) throws ParameterLimitException {
final OptionMap options = exchange.getConnection().getUndertowOptions();
boolean slashDecodingFlag = URLUtils.getSlashDecodingFlag(options);
setExchangeRequestPath(exchange, encodedPath,
options.get(UndertowOptions.URL_CHARSET, StandardCharsets.UTF_8.name()),
options.get(UndertowOptions.DECODE_URL, true),
options.get(UndertowOptions.ALLOW_ENCODED_SLASH, false),
slashDecodingFlag,
decodeBuffer,
options.get(UndertowOptions.MAX_PARAMETERS, UndertowOptions.DEFAULT_MAX_PARAMETERS));
}
Expand All @@ -476,7 +478,7 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin
* @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) throws ParameterLimitException {
public static void setExchangeRequestPath(final HttpServerExchange exchange, final String encodedPath, final String charset, boolean decode, final boolean decodeSlashFlag, StringBuilder decodeBuffer, int maxParameters) throws ParameterLimitException {
boolean requiresDecode = false;
final StringBuilder pathBuilder = new StringBuilder();
int currentPathPartIndex = 0;
Expand All @@ -486,7 +488,7 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin
String part;
String encodedPart = encodedPath.substring(currentPathPartIndex, i);
if (requiresDecode) {
part = URLUtils.decode(encodedPart, charset, allowEncodedSlash,false, decodeBuffer);
part = URLUtils.decode(encodedPart, charset, decodeSlashFlag,false, decodeBuffer);
} else {
part = encodedPart;
}
Expand All @@ -503,7 +505,7 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin
String part;
String encodedPart = encodedPath.substring(currentPathPartIndex, i);
if (requiresDecode) {
part = URLUtils.decode(encodedPart, charset, allowEncodedSlash, false, decodeBuffer);
part = URLUtils.decode(encodedPart, charset, decodeSlashFlag, false, decodeBuffer);
} else {
part = encodedPart;
}
Expand All @@ -519,7 +521,7 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin
String part;
String encodedPart = encodedPath.substring(currentPathPartIndex);
if (requiresDecode) {
part = URLUtils.decode(encodedPart, charset, allowEncodedSlash, false, decodeBuffer);
part = URLUtils.decode(encodedPart, charset, decodeSlashFlag, false, decodeBuffer);
} else {
part = encodedPart;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,10 @@ private static boolean shouldDecode(final HttpServerExchange exchange) {
}

private static void decodePath(HttpServerExchange exchange, String charset, StringBuilder sb) {
final boolean decodeSlash = exchange.getConnection().getUndertowOptions().get(UndertowOptions.ALLOW_ENCODED_SLASH, false);
exchange.setRequestPath(URLUtils.decode(exchange.getRequestPath(), charset, decodeSlash, false, sb));
exchange.setRelativePath(URLUtils.decode(exchange.getRelativePath(), charset, decodeSlash, false, sb));
exchange.setResolvedPath(URLUtils.decode(exchange.getResolvedPath(), charset, decodeSlash, false, sb));
boolean slashDecodingFlag = URLUtils.getSlashDecodingFlag(exchange.getConnection().getUndertowOptions());
exchange.setRequestPath(URLUtils.decode(exchange.getRequestPath(), charset, slashDecodingFlag, false, sb));
exchange.setRelativePath(URLUtils.decode(exchange.getRelativePath(), charset, slashDecodingFlag, false, sb));
exchange.setResolvedPath(URLUtils.decode(exchange.getResolvedPath(), charset, slashDecodingFlag, false, sb));
}

private static void decodeQueryString(HttpServerExchange exchange, String charset, StringBuilder sb) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import io.undertow.server.OpenListener;
import io.undertow.server.ServerConnection;
import io.undertow.server.XnioByteBufferPool;
import io.undertow.util.URLUtils;

import org.xnio.IoUtils;
import org.xnio.OptionMap;
import org.xnio.Options;
Expand Down Expand Up @@ -98,7 +100,7 @@ public AjpOpenListener(final ByteBufferPool pool, final OptionMap undertowOption
PooledByteBuffer buf = pool.allocate();
this.bufferSize = buf.getBuffer().remaining();
buf.close();
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), undertowOptions.get(UndertowOptions.ALLOW_ENCODED_SLASH, false), undertowOptions.get(UndertowOptions.ALLOW_UNESCAPED_CHARACTERS_IN_URL, false), undertowOptions.get(UndertowOptions.AJP_ALLOWED_REQUEST_ATTRIBUTES_PATTERN, DEFAULT_AJP_ALLOWED_REQUEST_ATTRIBUTES_PATTERN));
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), URLUtils.getSlashDecodingFlag(undertowOptions), undertowOptions.get(UndertowOptions.ALLOW_UNESCAPED_CHARACTERS_IN_URL, false), undertowOptions.get(UndertowOptions.AJP_ALLOWED_REQUEST_ATTRIBUTES_PATTERN, DEFAULT_AJP_ALLOWED_REQUEST_ATTRIBUTES_PATTERN));
connectorStatistics = new ConnectorStatisticsImpl();
statisticsEnabled = undertowOptions.get(UndertowOptions.ENABLE_CONNECTOR_STATISTICS, false);
}
Expand Down Expand Up @@ -178,7 +180,7 @@ public void setUndertowOptions(final OptionMap undertowOptions) {
}
this.undertowOptions = undertowOptions;
statisticsEnabled = undertowOptions.get(UndertowOptions.ENABLE_CONNECTOR_STATISTICS, false);
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), undertowOptions.get(UndertowOptions.ALLOW_ENCODED_SLASH, false), undertowOptions.get(UndertowOptions.ALLOW_UNESCAPED_CHARACTERS_IN_URL, false));
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), URLUtils.getSlashDecodingFlag(undertowOptions), undertowOptions.get(UndertowOptions.ALLOW_UNESCAPED_CHARACTERS_IN_URL, false));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public class AjpRequestParser {

private final String encoding;
private final boolean doDecode;
private final boolean allowEncodedSlash;
private final boolean slashDecodingFlag;
private final int maxParameters;
private final int maxHeaders;
private StringBuilder decodeBuffer;
Expand Down Expand Up @@ -184,16 +184,16 @@ public class AjpRequestParser {
ATTR_SET = new HashSet<>(Arrays.asList(ATTRIBUTES));
}

public AjpRequestParser(String encoding, boolean doDecode, int maxParameters, int maxHeaders, boolean allowEncodedSlash, boolean allowUnescapedCharactersInUrl) {
this(encoding, doDecode, maxParameters, maxHeaders, allowEncodedSlash, allowUnescapedCharactersInUrl, null);
public AjpRequestParser(String encoding, boolean doDecode, int maxParameters, int maxHeaders, boolean slashDecodingFlag, boolean allowUnescapedCharactersInUrl) {
this(encoding, doDecode, maxParameters, maxHeaders, slashDecodingFlag, allowUnescapedCharactersInUrl, null);
}

public AjpRequestParser(String encoding, boolean doDecode, int maxParameters, int maxHeaders, boolean allowEncodedSlash, boolean allowUnescapedCharactersInUrl, String allowedRequestAttributesPattern) {
public AjpRequestParser(String encoding, boolean doDecode, int maxParameters, int maxHeaders, boolean slashDecodingFlag, boolean allowUnescapedCharactersInUrl, String allowedRequestAttributesPattern) {
this.encoding = encoding;
this.doDecode = doDecode;
this.maxParameters = maxParameters;
this.maxHeaders = maxHeaders;
this.allowEncodedSlash = allowEncodedSlash;
this.slashDecodingFlag = slashDecodingFlag;
this.allowUnescapedCharactersInUrl = allowUnescapedCharactersInUrl;
if (allowedRequestAttributesPattern != null && !allowedRequestAttributesPattern.isEmpty()) {
this.allowedRequestAttributesPattern = Pattern.compile(allowedRequestAttributesPattern);
Expand Down Expand Up @@ -512,7 +512,7 @@ private String decode(String url, final boolean containsUrlCharacters) throws Un
if(decodeBuffer == null) {
decodeBuffer = new StringBuilder();
}
return URLUtils.decode(url, this.encoding, allowEncodedSlash, false, decodeBuffer);
return URLUtils.decode(url, this.encoding, slashDecodingFlag, false, decodeBuffer);
} catch (Exception e) {
throw UndertowMessages.MESSAGES.failedToDecodeURL(url, encoding, e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ public abstract class HttpRequestParser {

private final int maxParameters;
private final int maxHeaders;
private final boolean allowEncodedSlash;
private final boolean slashDecodingFlag;
private final boolean decode;
private final String charset;
private final int maxCachedHeaderSize;
Expand Down Expand Up @@ -212,7 +212,7 @@ public static boolean isTargetCharacterAllowed(char c) {
public HttpRequestParser(OptionMap options) {
maxParameters = options.get(UndertowOptions.MAX_PARAMETERS, UndertowOptions.DEFAULT_MAX_PARAMETERS);
maxHeaders = options.get(UndertowOptions.MAX_HEADERS, UndertowOptions.DEFAULT_MAX_HEADERS);
allowEncodedSlash = options.get(UndertowOptions.ALLOW_ENCODED_SLASH, false);
slashDecodingFlag = URLUtils.getSlashDecodingFlag(options);
decode = options.get(UndertowOptions.DECODE_URL, true);
charset = options.get(UndertowOptions.URL_CHARSET, StandardCharsets.UTF_8.name());
maxCachedHeaderSize = options.get(UndertowOptions.MAX_CACHED_HEADER_SIZE, UndertowOptions.DEFAULT_MAX_CACHED_HEADER_SIZE);
Expand Down Expand Up @@ -474,7 +474,7 @@ private void parsePathComplete(ParseState state, HttpServerExchange exchange, in
exchange.setRelativePath("/");
exchange.setRequestURI(path, true);
} else if (parseState < HOST_DONE && state.canonicalPath.length() == 0) {
String decodedPath = decode(path, urlDecodeRequired, state, allowEncodedSlash, false);
String decodedPath = decode(path, urlDecodeRequired, state, slashDecodingFlag, false);
exchange.setRequestPath(decodedPath);
exchange.setRelativePath(decodedPath);
exchange.setRequestURI(path, false);
Expand All @@ -497,7 +497,7 @@ private void beginQueryParameters(ByteBuffer buffer, ParseState state, HttpServe

private void handleFullUrl(ParseState state, HttpServerExchange exchange, int canonicalPathStart, boolean urlDecodeRequired, String path, int parseState) {
state.canonicalPath.append(path.substring(canonicalPathStart));
String thePath = decode(state.canonicalPath.toString(), urlDecodeRequired, state, allowEncodedSlash, false);
String thePath = decode(state.canonicalPath.toString(), urlDecodeRequired, state, slashDecodingFlag, false);
exchange.setRequestPath(thePath);
exchange.setRelativePath(thePath);
exchange.setRequestURI(path, parseState == HOST_DONE);
Expand Down Expand Up @@ -587,9 +587,9 @@ final void handleQueryParameters(ByteBuffer buffer, ParseState state, HttpServer
state.mapCount = mapCount;
}

private String decode(final String value, boolean urlDecodeRequired, ParseState state, final boolean allowEncodedSlash, final boolean formEncoded) {
private String decode(final String value, boolean urlDecodeRequired, ParseState state, final boolean slashDecodingFlag, final boolean formEncoded) {
if (urlDecodeRequired) {
return URLUtils.decode(value, charset, allowEncodedSlash, formEncoded, state.decodeBuffer);
return URLUtils.decode(value, charset, slashDecodingFlag, formEncoded, state.decodeBuffer);
} else {
return value;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@
import io.undertow.util.ParameterLimitException;
import io.undertow.util.Protocols;
import io.undertow.util.StatusCodes;
import io.undertow.util.URLUtils;

import org.xnio.ChannelListener;
import org.xnio.IoUtils;
import org.xnio.OptionMap;
Expand All @@ -78,7 +80,7 @@ public class Http2ReceiveListener implements ChannelListener<Http2Channel> {
private final String encoding;
private final boolean decode;
private final StringBuilder decodeBuffer = new StringBuilder();
private final boolean allowEncodingSlash;
private final boolean slashDecodingFlag;
private final int bufferSize;
private final int maxParameters;
private final boolean recordRequestStartTime;
Expand All @@ -92,7 +94,7 @@ public Http2ReceiveListener(HttpHandler rootHandler, OptionMap undertowOptions,
this.bufferSize = bufferSize;
this.connectorStatistics = connectorStatistics;
this.maxEntitySize = undertowOptions.get(UndertowOptions.MAX_ENTITY_SIZE, UndertowOptions.DEFAULT_MAX_ENTITY_SIZE);
this.allowEncodingSlash = undertowOptions.get(UndertowOptions.ALLOW_ENCODED_SLASH, false);
this.slashDecodingFlag = URLUtils.getSlashDecodingFlag(undertowOptions);
this.decode = undertowOptions.get(UndertowOptions.DECODE_URL, true);
this.maxParameters = undertowOptions.get(UndertowOptions.MAX_PARAMETERS, UndertowOptions.DEFAULT_MAX_PARAMETERS);
this.recordRequestStartTime = undertowOptions.get(UndertowOptions.RECORD_REQUEST_START_TIME, false);
Expand Down Expand Up @@ -190,7 +192,7 @@ public void handleEvent(Http2StreamSourceChannel channel) {
}

try {
Connectors.setExchangeRequestPath(exchange, path, encoding, decode, allowEncodingSlash, decodeBuffer, maxParameters);
Connectors.setExchangeRequestPath(exchange, path, encoding, decode, slashDecodingFlag, decodeBuffer, maxParameters);
} catch (ParameterLimitException e) {
//this can happen if max parameters is exceeded
UndertowLogger.REQUEST_IO_LOGGER.debug("Failed to set request path", e);
Expand Down Expand Up @@ -241,7 +243,7 @@ void handleInitialRequest(HttpServerExchange initial, Http2Channel channel, byte
Connectors.terminateRequest(exchange);
String uri = exchange.getQueryString().isEmpty() ? initial.getRequestURI() : initial.getRequestURI() + '?' + exchange.getQueryString();
try {
Connectors.setExchangeRequestPath(exchange, uri, encoding, decode, allowEncodingSlash, decodeBuffer, maxParameters);
Connectors.setExchangeRequestPath(exchange, uri, encoding, decode, slashDecodingFlag, decodeBuffer, maxParameters);
} catch (ParameterLimitException e) {
exchange.setStatusCode(StatusCodes.BAD_REQUEST);
exchange.endExchange();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
import io.undertow.util.HeaderMap;
import io.undertow.util.HttpString;
import io.undertow.util.StatusCodes;
import io.undertow.util.URLUtils;

import static io.undertow.protocols.http2.Http2Channel.AUTHORITY;
import static io.undertow.protocols.http2.Http2Channel.METHOD;
Expand Down Expand Up @@ -440,7 +441,7 @@ public boolean pushResource(String path, HttpString method, HeaderMap requestHea
exchange.setProtocol(Protocols.HTTP_1_1);
exchange.setRequestScheme(this.exchange.getRequestScheme());
try {
Connectors.setExchangeRequestPath(exchange, path, getUndertowOptions().get(UndertowOptions.URL_CHARSET, StandardCharsets.UTF_8.name()), getUndertowOptions().get(UndertowOptions.DECODE_URL, true), getUndertowOptions().get(UndertowOptions.ALLOW_ENCODED_SLASH, false), new StringBuilder(), getUndertowOptions().get(UndertowOptions.MAX_PARAMETERS, UndertowOptions.DEFAULT_MAX_HEADERS));
Connectors.setExchangeRequestPath(exchange, path, getUndertowOptions().get(UndertowOptions.URL_CHARSET, StandardCharsets.UTF_8.name()), getUndertowOptions().get(UndertowOptions.DECODE_URL, true), URLUtils.getSlashDecodingFlag(getUndertowOptions()), new StringBuilder(), getUndertowOptions().get(UndertowOptions.MAX_PARAMETERS, UndertowOptions.DEFAULT_MAX_HEADERS));
} catch (ParameterLimitException e) {
UndertowLogger.REQUEST_IO_LOGGER.debug("Too many parameters in HTTP/2 request", e);
exchange.setStatusCode(StatusCodes.BAD_REQUEST);
Expand Down
Loading

0 comments on commit 6da238e

Please sign in to comment.