Skip to content

Commit 1048d26

Browse files
authored
Merge pull request #1444 from soul2zimate/JBEAP-24234-2.2.x
[UNDERTOW-2200] - Path and query parameters are not decoded properly …
2 parents 7b5c25a + 9298f32 commit 1048d26

File tree

11 files changed

+131
-30
lines changed

11 files changed

+131
-30
lines changed

core/src/main/java/io/undertow/UndertowOptions.java

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,11 +117,25 @@ public class UndertowOptions {
117117
* this is disabled by default.
118118
* <p>
119119
* Defaults to false
120-
*
120+
* <p>
121121
* See <a href="http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2007-0450">CVE-2007-0450</a>
122+
* @deprecated - this option was interpreted improperly.
123+
* @See {@link #DECODE_SLASH}
122124
*/
123125
public static final Option<Boolean> ALLOW_ENCODED_SLASH = Option.simple(UndertowOptions.class, "ALLOW_ENCODED_SLASH", Boolean.class);
124126

127+
/**
128+
* If a request comes in with encoded / characters (i.e. %2F), will these be decoded.
129+
* <p>
130+
* This can cause security problems if a front end proxy does not perform the same decoding, and as a result
131+
* this is disabled by default.
132+
* <p>
133+
* If this option is set explicitly, the {@link #ALLOW_ENCODED_SLASH} is ignored. Should default to <b>true</b>
134+
* <p>
135+
* See <a href="http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2007-0450">CVE-2007-0450</a>
136+
*/
137+
public static final Option<Boolean> DECODE_SLASH = Option.simple(UndertowOptions.class, "DECODE_SLASH", Boolean.class);
138+
125139
/**
126140
* 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
127141
* not be decoded. This will allow a later handler to decode them into whatever charset is desired.

core/src/main/java/io/undertow/server/Connectors.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,8 @@ public static void executeRootHandler(final HttpHandler handler, final HttpServe
444444
@Deprecated
445445
public static void setExchangeRequestPath(final HttpServerExchange exchange, final String encodedPath, final String charset, boolean decode, final boolean allowEncodedSlash, StringBuilder decodeBuffer) {
446446
try {
447-
setExchangeRequestPath(exchange, encodedPath, charset, decode, allowEncodedSlash, decodeBuffer, exchange.getConnection().getUndertowOptions().get(UndertowOptions.MAX_PARAMETERS, UndertowOptions.DEFAULT_MAX_PARAMETERS));
447+
final boolean slashDecodingFlag = URLUtils.getSlashDecodingFlag(allowEncodedSlash, exchange.getConnection().getUndertowOptions().get(UndertowOptions.DECODE_SLASH));
448+
setExchangeRequestPath(exchange, encodedPath, charset, decode, slashDecodingFlag, decodeBuffer, exchange.getConnection().getUndertowOptions().get(UndertowOptions.MAX_PARAMETERS, UndertowOptions.DEFAULT_MAX_PARAMETERS));
448449
} catch (ParameterLimitException e) {
449450
throw new RuntimeException(e);
450451
}
@@ -461,10 +462,11 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin
461462
*/
462463
public static void setExchangeRequestPath(final HttpServerExchange exchange, final String encodedPath, StringBuilder decodeBuffer) throws ParameterLimitException {
463464
final OptionMap options = exchange.getConnection().getUndertowOptions();
465+
boolean slashDecodingFlag = URLUtils.getSlashDecodingFlag(options);
464466
setExchangeRequestPath(exchange, encodedPath,
465467
options.get(UndertowOptions.URL_CHARSET, StandardCharsets.UTF_8.name()),
466468
options.get(UndertowOptions.DECODE_URL, true),
467-
options.get(UndertowOptions.ALLOW_ENCODED_SLASH, false),
469+
slashDecodingFlag,
468470
decodeBuffer,
469471
options.get(UndertowOptions.MAX_PARAMETERS, UndertowOptions.DEFAULT_MAX_PARAMETERS));
470472
}
@@ -476,7 +478,7 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin
476478
* @param encodedPath The encoded path
477479
* @param charset The charset
478480
*/
479-
public static void setExchangeRequestPath(final HttpServerExchange exchange, final String encodedPath, final String charset, boolean decode, final boolean allowEncodedSlash, StringBuilder decodeBuffer, int maxParameters) throws ParameterLimitException {
481+
public static void setExchangeRequestPath(final HttpServerExchange exchange, final String encodedPath, final String charset, boolean decode, final boolean decodeSlashFlag, StringBuilder decodeBuffer, int maxParameters) throws ParameterLimitException {
480482
boolean requiresDecode = false;
481483
final StringBuilder pathBuilder = new StringBuilder();
482484
int currentPathPartIndex = 0;
@@ -486,7 +488,7 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin
486488
String part;
487489
String encodedPart = encodedPath.substring(currentPathPartIndex, i);
488490
if (requiresDecode) {
489-
part = URLUtils.decode(encodedPart, charset, allowEncodedSlash,false, decodeBuffer);
491+
part = URLUtils.decode(encodedPart, charset, decodeSlashFlag,false, decodeBuffer);
490492
} else {
491493
part = encodedPart;
492494
}
@@ -503,7 +505,7 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin
503505
String part;
504506
String encodedPart = encodedPath.substring(currentPathPartIndex, i);
505507
if (requiresDecode) {
506-
part = URLUtils.decode(encodedPart, charset, allowEncodedSlash, false, decodeBuffer);
508+
part = URLUtils.decode(encodedPart, charset, decodeSlashFlag, false, decodeBuffer);
507509
} else {
508510
part = encodedPart;
509511
}
@@ -519,7 +521,7 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin
519521
String part;
520522
String encodedPart = encodedPath.substring(currentPathPartIndex);
521523
if (requiresDecode) {
522-
part = URLUtils.decode(encodedPart, charset, allowEncodedSlash, false, decodeBuffer);
524+
part = URLUtils.decode(encodedPart, charset, decodeSlashFlag, false, decodeBuffer);
523525
} else {
524526
part = encodedPart;
525527
}

core/src/main/java/io/undertow/server/handlers/URLDecodingHandler.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,10 @@ private static boolean shouldDecode(final HttpServerExchange exchange) {
7676
}
7777

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

8585
private static void decodeQueryString(HttpServerExchange exchange, String charset, StringBuilder sb) {

core/src/main/java/io/undertow/server/protocol/ajp/AjpOpenListener.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
import io.undertow.server.OpenListener;
3333
import io.undertow.server.ServerConnection;
3434
import io.undertow.server.XnioByteBufferPool;
35+
import io.undertow.util.URLUtils;
36+
3537
import org.xnio.IoUtils;
3638
import org.xnio.OptionMap;
3739
import org.xnio.Options;
@@ -98,7 +100,7 @@ public AjpOpenListener(final ByteBufferPool pool, final OptionMap undertowOption
98100
PooledByteBuffer buf = pool.allocate();
99101
this.bufferSize = buf.getBuffer().remaining();
100102
buf.close();
101-
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));
103+
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));
102104
connectorStatistics = new ConnectorStatisticsImpl();
103105
statisticsEnabled = undertowOptions.get(UndertowOptions.ENABLE_CONNECTOR_STATISTICS, false);
104106
}
@@ -178,7 +180,7 @@ public void setUndertowOptions(final OptionMap undertowOptions) {
178180
}
179181
this.undertowOptions = undertowOptions;
180182
statisticsEnabled = undertowOptions.get(UndertowOptions.ENABLE_CONNECTOR_STATISTICS, false);
181-
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));
183+
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));
182184
}
183185

184186
@Override

core/src/main/java/io/undertow/server/protocol/ajp/AjpRequestParser.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public class AjpRequestParser {
7474

7575
private final String encoding;
7676
private final boolean doDecode;
77-
private final boolean allowEncodedSlash;
77+
private final boolean slashDecodingFlag;
7878
private final int maxParameters;
7979
private final int maxHeaders;
8080
private StringBuilder decodeBuffer;
@@ -184,16 +184,16 @@ public class AjpRequestParser {
184184
ATTR_SET = new HashSet<String>(Arrays.asList(ATTRIBUTES));
185185
}
186186

187-
public AjpRequestParser(String encoding, boolean doDecode, int maxParameters, int maxHeaders, boolean allowEncodedSlash, boolean allowUnescapedCharactersInUrl) {
188-
this(encoding, doDecode, maxParameters, maxHeaders, allowEncodedSlash, allowUnescapedCharactersInUrl, null);
187+
public AjpRequestParser(String encoding, boolean doDecode, int maxParameters, int maxHeaders, boolean slashDecodingFlag, boolean allowUnescapedCharactersInUrl) {
188+
this(encoding, doDecode, maxParameters, maxHeaders, slashDecodingFlag, allowUnescapedCharactersInUrl, null);
189189
}
190190

191-
public AjpRequestParser(String encoding, boolean doDecode, int maxParameters, int maxHeaders, boolean allowEncodedSlash, boolean allowUnescapedCharactersInUrl, String allowedRequestAttributesPattern) {
191+
public AjpRequestParser(String encoding, boolean doDecode, int maxParameters, int maxHeaders, boolean slashDecodingFlag, boolean allowUnescapedCharactersInUrl, String allowedRequestAttributesPattern) {
192192
this.encoding = encoding;
193193
this.doDecode = doDecode;
194194
this.maxParameters = maxParameters;
195195
this.maxHeaders = maxHeaders;
196-
this.allowEncodedSlash = allowEncodedSlash;
196+
this.slashDecodingFlag = slashDecodingFlag;
197197
this.allowUnescapedCharactersInUrl = allowUnescapedCharactersInUrl;
198198
if (allowedRequestAttributesPattern != null && !allowedRequestAttributesPattern.isEmpty()) {
199199
this.allowedRequestAttributesPattern = Pattern.compile(allowedRequestAttributesPattern);
@@ -512,7 +512,7 @@ private String decode(String url, final boolean containsUrlCharacters) throws Un
512512
if(decodeBuffer == null) {
513513
decodeBuffer = new StringBuilder();
514514
}
515-
return URLUtils.decode(url, this.encoding, allowEncodedSlash, false, decodeBuffer);
515+
return URLUtils.decode(url, this.encoding, slashDecodingFlag, false, decodeBuffer);
516516
} catch (Exception e) {
517517
throw UndertowMessages.MESSAGES.failedToDecodeURL(url, encoding, e);
518518
}

core/src/main/java/io/undertow/server/protocol/http/HttpRequestParser.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ public abstract class HttpRequestParser {
162162

163163
private final int maxParameters;
164164
private final int maxHeaders;
165-
private final boolean allowEncodedSlash;
165+
private final boolean slashDecodingFlag;
166166
private final boolean decode;
167167
private final String charset;
168168
private final int maxCachedHeaderSize;
@@ -208,7 +208,7 @@ public static boolean isTargetCharacterAllowed(char c) {
208208
public HttpRequestParser(OptionMap options) {
209209
maxParameters = options.get(UndertowOptions.MAX_PARAMETERS, UndertowOptions.DEFAULT_MAX_PARAMETERS);
210210
maxHeaders = options.get(UndertowOptions.MAX_HEADERS, UndertowOptions.DEFAULT_MAX_HEADERS);
211-
allowEncodedSlash = options.get(UndertowOptions.ALLOW_ENCODED_SLASH, false);
211+
slashDecodingFlag = URLUtils.getSlashDecodingFlag(options);
212212
decode = options.get(UndertowOptions.DECODE_URL, true);
213213
charset = options.get(UndertowOptions.URL_CHARSET, StandardCharsets.UTF_8.name());
214214
maxCachedHeaderSize = options.get(UndertowOptions.MAX_CACHED_HEADER_SIZE, UndertowOptions.DEFAULT_MAX_CACHED_HEADER_SIZE);
@@ -458,7 +458,7 @@ private void parsePathComplete(ParseState state, HttpServerExchange exchange, in
458458
exchange.setRelativePath("/");
459459
exchange.setRequestURI(path, true);
460460
} else if (parseState < HOST_DONE && state.canonicalPath.length() == 0) {
461-
String decodedPath = decode(path, urlDecodeRequired, state, allowEncodedSlash, false);
461+
String decodedPath = decode(path, urlDecodeRequired, state, slashDecodingFlag, false);
462462
exchange.setRequestPath(decodedPath);
463463
exchange.setRelativePath(decodedPath);
464464
exchange.setRequestURI(path, false);
@@ -481,7 +481,7 @@ private void beginQueryParameters(ByteBuffer buffer, ParseState state, HttpServe
481481

482482
private void handleFullUrl(ParseState state, HttpServerExchange exchange, int canonicalPathStart, boolean urlDecodeRequired, String path, int parseState) {
483483
state.canonicalPath.append(path.substring(canonicalPathStart));
484-
String thePath = decode(state.canonicalPath.toString(), urlDecodeRequired, state, allowEncodedSlash, false);
484+
String thePath = decode(state.canonicalPath.toString(), urlDecodeRequired, state, slashDecodingFlag, false);
485485
exchange.setRequestPath(thePath);
486486
exchange.setRelativePath(thePath);
487487
exchange.setRequestURI(path, parseState == HOST_DONE);
@@ -571,9 +571,9 @@ final void handleQueryParameters(ByteBuffer buffer, ParseState state, HttpServer
571571
state.mapCount = mapCount;
572572
}
573573

574-
private String decode(final String value, boolean urlDecodeRequired, ParseState state, final boolean allowEncodedSlash, final boolean formEncoded) {
574+
private String decode(final String value, boolean urlDecodeRequired, ParseState state, final boolean slashDecodingFlag, final boolean formEncoded) {
575575
if (urlDecodeRequired) {
576-
return URLUtils.decode(value, charset, allowEncodedSlash, formEncoded, state.decodeBuffer);
576+
return URLUtils.decode(value, charset, slashDecodingFlag, formEncoded, state.decodeBuffer);
577577
} else {
578578
return value;
579579
}

core/src/main/java/io/undertow/server/protocol/http2/Http2ReceiveListener.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@
5252
import io.undertow.util.ParameterLimitException;
5353
import io.undertow.util.Protocols;
5454
import io.undertow.util.StatusCodes;
55+
import io.undertow.util.URLUtils;
56+
5557
import org.xnio.ChannelListener;
5658
import org.xnio.IoUtils;
5759
import org.xnio.OptionMap;
@@ -78,7 +80,7 @@ public class Http2ReceiveListener implements ChannelListener<Http2Channel> {
7880
private final String encoding;
7981
private final boolean decode;
8082
private final StringBuilder decodeBuffer = new StringBuilder();
81-
private final boolean allowEncodingSlash;
83+
private final boolean slashDecodingFlag;
8284
private final int bufferSize;
8385
private final int maxParameters;
8486
private final boolean recordRequestStartTime;
@@ -92,7 +94,7 @@ public Http2ReceiveListener(HttpHandler rootHandler, OptionMap undertowOptions,
9294
this.bufferSize = bufferSize;
9395
this.connectorStatistics = connectorStatistics;
9496
this.maxEntitySize = undertowOptions.get(UndertowOptions.MAX_ENTITY_SIZE, UndertowOptions.DEFAULT_MAX_ENTITY_SIZE);
95-
this.allowEncodingSlash = undertowOptions.get(UndertowOptions.ALLOW_ENCODED_SLASH, false);
97+
this.slashDecodingFlag = URLUtils.getSlashDecodingFlag(undertowOptions);
9698
this.decode = undertowOptions.get(UndertowOptions.DECODE_URL, true);
9799
this.maxParameters = undertowOptions.get(UndertowOptions.MAX_PARAMETERS, UndertowOptions.DEFAULT_MAX_PARAMETERS);
98100
this.recordRequestStartTime = undertowOptions.get(UndertowOptions.RECORD_REQUEST_START_TIME, false);
@@ -190,7 +192,7 @@ public void handleEvent(Http2StreamSourceChannel channel) {
190192
}
191193

192194
try {
193-
Connectors.setExchangeRequestPath(exchange, path, encoding, decode, allowEncodingSlash, decodeBuffer, maxParameters);
195+
Connectors.setExchangeRequestPath(exchange, path, encoding, decode, slashDecodingFlag, decodeBuffer, maxParameters);
194196
} catch (ParameterLimitException e) {
195197
//this can happen if max parameters is exceeded
196198
UndertowLogger.REQUEST_IO_LOGGER.debug("Failed to set request path", e);
@@ -241,7 +243,7 @@ void handleInitialRequest(HttpServerExchange initial, Http2Channel channel, byte
241243
Connectors.terminateRequest(exchange);
242244
String uri = exchange.getQueryString().isEmpty() ? initial.getRequestURI() : initial.getRequestURI() + '?' + exchange.getQueryString();
243245
try {
244-
Connectors.setExchangeRequestPath(exchange, uri, encoding, decode, allowEncodingSlash, decodeBuffer, maxParameters);
246+
Connectors.setExchangeRequestPath(exchange, uri, encoding, decode, slashDecodingFlag, decodeBuffer, maxParameters);
245247
} catch (ParameterLimitException e) {
246248
exchange.setStatusCode(StatusCodes.BAD_REQUEST);
247249
exchange.endExchange();

core/src/main/java/io/undertow/server/protocol/http2/Http2ServerConnection.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
import io.undertow.util.HeaderMap;
7373
import io.undertow.util.HttpString;
7474
import io.undertow.util.StatusCodes;
75+
import io.undertow.util.URLUtils;
7576

7677
import static io.undertow.protocols.http2.Http2Channel.AUTHORITY;
7778
import static io.undertow.protocols.http2.Http2Channel.METHOD;
@@ -432,7 +433,7 @@ public boolean pushResource(String path, HttpString method, HeaderMap requestHea
432433
exchange.setProtocol(Protocols.HTTP_1_1);
433434
exchange.setRequestScheme(this.exchange.getRequestScheme());
434435
try {
435-
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));
436+
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));
436437
} catch (ParameterLimitException e) {
437438
UndertowLogger.REQUEST_IO_LOGGER.debug("Too many parameters in HTTP/2 request", e);
438439
exchange.setStatusCode(StatusCodes.BAD_REQUEST);

0 commit comments

Comments
 (0)