High severityNVD Advisory· Published Jul 27, 2018· Updated Aug 5, 2024
CVE-2017-12165
CVE-2017-12165
Description
It was discovered that Undertow before 1.4.17, 1.3.31 and 2.0.0 processes http request headers with unusual whitespaces which can cause possible http request smuggling.
Affected packages
Versions sourced from the GitHub Security Advisory.
| Package | Affected versions | Patched versions |
|---|---|---|
io.undertow:undertow-coreMaven | < 1.3.31 | 1.3.31 |
io.undertow:undertow-coreMaven | >= 1.4.0, < 1.4.17 | 1.4.17 |
io.undertow:undertow-coreMaven | >= 2.0.0.Alpha1, < 2.0.0.Beta1 | 2.0.0.Beta1 |
Affected products
1Patches
35b008b7ac312JBEAP-9909 Prevent HTTP smuggling attacks by making sure messages do not contain invalid headers.
14 files changed · +302 −15
core/src/main/java/io/undertow/protocols/http2/Http2HeaderBlockParser.java+4 −0 modified@@ -28,6 +28,7 @@ import io.undertow.UndertowLogger; import io.undertow.UndertowMessages; +import io.undertow.server.Connectors; import io.undertow.util.HeaderMap; import io.undertow.util.Headers; import io.undertow.util.HttpString; @@ -177,6 +178,9 @@ public void emitHeader(HttpString name, String value, boolean neverIndex) throws if(c>= 'A' && c <= 'Z') { invalid = true; UndertowLogger.REQUEST_LOGGER.debugf("Malformed request, header %s contains uppercase characters", name); + } else if(c != ':' && !Connectors.isValidTokenCharacter(c)) { + invalid = true; + UndertowLogger.REQUEST_LOGGER.debugf("Malformed request, header %s contains invalid token character", name); } }
core/src/main/java/io/undertow/server/Connectors.java+76 −0 modified@@ -19,10 +19,14 @@ package io.undertow.server; import io.undertow.UndertowLogger; +import io.undertow.UndertowMessages; import io.undertow.UndertowOptions; import io.undertow.server.handlers.Cookie; import io.undertow.util.DateUtils; +import io.undertow.util.HeaderMap; +import io.undertow.util.HeaderValues; import io.undertow.util.Headers; +import io.undertow.util.HttpString; import io.undertow.util.ParameterLimitException; import io.undertow.util.StatusCodes; import io.undertow.util.URLUtils; @@ -46,7 +50,40 @@ */ public class Connectors { + private static final boolean[] ALLOWED_TOKEN_CHARACTERS = new boolean[256]; + static { + for(int i = 0; i < ALLOWED_TOKEN_CHARACTERS.length; ++i) { + if((i >='0' && i <= '9') || + (i >='a' && i <= 'z') || + (i >='A' && i <= 'Z')) { + ALLOWED_TOKEN_CHARACTERS[i] = true; + } else { + switch (i) { + case '!': + case '#': + case '$': + case '%': + case '&': + case '\'': + case '*': + case '+': + case '-': + case '.': + case '^': + case '_': + case '`': + case '|': + case '~': { + ALLOWED_TOKEN_CHARACTERS[i] = true; + break; + } + default: + ALLOWED_TOKEN_CHARACTERS[i] = false; + } + } + } + } /** * Flattens the exchange cookie map into the response header map. This should be called by a * connector just before the response is started. @@ -363,4 +400,43 @@ public static void updateResponseBytesSent(HttpServerExchange exchange, long byt public static ConduitStreamSinkChannel getConduitSinkChannel(HttpServerExchange exchange) { return exchange.getConnection().getSinkChannel(); } + + /** + * Verifies that the contents of the HttpString are a valid token according to rfc7230. + * @param header The header to verify + */ + public static void verifyToken(HttpString header) { + int length = header.length(); + for(int i = 0; i < length; ++i) { + byte c = header.byteAt(i); + if(!ALLOWED_TOKEN_CHARACTERS[c]) { + throw UndertowMessages.MESSAGES.invalidToken(c); + } + } + } + + /** + * Returns true if the token character is valid according to rfc7230 + */ + public static boolean isValidTokenCharacter(byte c) { + return ALLOWED_TOKEN_CHARACTERS[c]; + } + + + /** + * Verifies that the provided request headers are valid according to rfc7230. In particular: + * - At most one content-length or transfer encoding + */ + public static boolean areRequestHeadersValid(HeaderMap headers) { + HeaderValues te = headers.get(Headers.TRANSFER_ENCODING); + HeaderValues cl = headers.get(Headers.CONTENT_LENGTH); + if(te != null && cl != null) { + return false; + } else if(te != null && te.size() > 1) { + return false; + } else if(cl != null && cl.size() > 1) { + return false; + } + return true; + } }
core/src/main/java/io/undertow/server/protocol/ajp/AjpReadListener.java+4 −0 modified@@ -238,6 +238,10 @@ public void handleEvent(AjpServerResponseConduit channel) { if(connectorStatistics != null) { connectorStatistics.setup(httpServerExchange); } + if(!Connectors.areRequestHeadersValid(httpServerExchange.getRequestHeaders())) { + oldState.badRequest = true; + UndertowLogger.REQUEST_IO_LOGGER.debugf("Invalid AJP request from %s, request contained invalid headers", connection.getPeerAddress()); + } if(oldState.badRequest) { httpServerExchange.setStatusCode(StatusCodes.BAD_REQUEST);
core/src/main/java/io/undertow/server/protocol/ajp/AjpRequestParser.java+5 −1 modified@@ -55,6 +55,7 @@ import io.undertow.UndertowLogger; import io.undertow.UndertowMessages; import io.undertow.security.impl.ExternalAuthenticationMechanism; +import io.undertow.server.Connectors; import io.undertow.server.HttpServerExchange; import io.undertow.util.Headers; import io.undertow.util.HttpString; @@ -345,6 +346,7 @@ public void parse(final ByteBuffer buf, final AjpRequestParseState state, final state.currentHeader = result.header; } else { state.currentHeader = HttpString.tryFromString(result.value); + Connectors.verifyToken(state.currentHeader); } } StringHolder result = parseString(buf, state, StringType.OTHER); @@ -423,7 +425,9 @@ public void parse(final ByteBuffer buf, final AjpRequestParseState state, final } else if (state.currentAttribute.equals(AUTH_TYPE)) { exchange.putAttachment(ExternalAuthenticationMechanism.EXTERNAL_AUTHENTICATION_TYPE, result); } else if (state.currentAttribute.equals(STORED_METHOD)) { - exchange.setRequestMethod(new HttpString(result)); + HttpString requestMethod = new HttpString(result); + Connectors.verifyToken(requestMethod); + exchange.setRequestMethod(requestMethod); } else if (state.currentAttribute.equals(AJP_REMOTE_PORT)) { state.remotePort = Integer.parseInt(result); } else if (state.currentAttribute.equals(SSL_SESSION)) {
core/src/main/java/io/undertow/server/protocol/http2/Http2ReceiveListener.java+5 −0 modified@@ -145,6 +145,11 @@ private void handleRequests(Http2Channel channel, Http2StreamSourceChannel frame exchange.setProtocol(Protocols.HTTP_2_0); exchange.setRequestMethod(Methods.fromString(exchange.getRequestHeaders().getFirst(METHOD))); exchange.getRequestHeaders().put(Headers.HOST, exchange.getRequestHeaders().getFirst(AUTHORITY)); + if(!Connectors.areRequestHeadersValid(exchange.getRequestHeaders())) { + UndertowLogger.REQUEST_IO_LOGGER.debugf("Invalid headers in HTTP/2 request, closing connection. Remote peer %s", connection.getPeerAddress()); + channel.sendGoAway(Http2Channel.ERROR_PROTOCOL_ERROR); + return; + } final String path = exchange.getRequestHeaders().getFirst(PATH); if(path == null || path.isEmpty()) {
core/src/main/java/io/undertow/server/protocol/http/HttpReadListener.java+4 −0 modified@@ -241,6 +241,10 @@ public void handleEventWithNoRunningRequest(final ConduitStreamSourceChannel cha return; } } + if(!Connectors.areRequestHeadersValid(httpServerExchange.getRequestHeaders())) { + sendBadRequestAndClose(connection.getChannel(), UndertowMessages.MESSAGES.invalidHeaders()); + return; + } Connectors.executeRootHandler(connection.getRootHandler(), httpServerExchange); } catch (Exception e) { sendBadRequestAndClose(connection.getChannel(), e);
core/src/main/java/io/undertow/server/protocol/http/HttpRequestParser.java+27 −0 modified@@ -167,13 +167,37 @@ public abstract class HttpRequestParser { private final String charset; private final int maxCachedHeaderSize; + private static final boolean[] ALLOWED_TARGET_CHARACTER = new boolean[256]; + static { try { HTTP = "HTTP/1.".getBytes("ASCII"); HTTP_LENGTH = HTTP.length; } catch (UnsupportedEncodingException e) { throw new RuntimeException(e); } + for(int i = 0; i < 256; ++i) { + if(i < 32 || i > 126) { + ALLOWED_TARGET_CHARACTER[i] = false; + } else { + switch ((char)i) { + case '\"': + case '#': + case '<': + case '>': + case '\\': + case '^': + case '`': + case '{': + case '|': + case '}': + ALLOWED_TARGET_CHARACTER[i] = false; + break; + default: + ALLOWED_TARGET_CHARACTER[i] = true; + } + } + } } public HttpRequestParser(OptionMap options) { @@ -348,6 +372,9 @@ final void handlePath(ByteBuffer buffer, ParseState state, HttpServerExchange ex while (buffer.hasRemaining()) { char next = (char) (buffer.get() & 0xFF); + if(!ALLOWED_TARGET_CHARACTER[next]) { + throw new BadRequestException(UndertowMessages.MESSAGES.invalidCharacterInRequestTarget(next)); + } if (next == ' ' || next == '\t') { if (stringBuilder.length() != 0) { final String path = stringBuilder.toString();
core/src/main/java/io/undertow/UndertowMessages.java+12 −0 modified@@ -513,4 +513,16 @@ public interface UndertowMessages { @Message(id = 161, value = "HTTP/2 header block is too large") String headerBlockTooLarge(); + + @Message(id = 162, value = "Same-site attribute %s is invalid. It must be Strict or Lax") + IllegalArgumentException invalidSameSiteMode(String mode); + + @Message(id = 163, value = "Invalid token %s") + IllegalArgumentException invalidToken(byte c); + + @Message(id = 164, value = "Request contained invalid headers") + IllegalArgumentException invalidHeaders(); + + @Message(id = 165, value = "Invalid character %s in request-target") + String invalidCharacterInRequestTarget(char next); }
core/src/main/java/io/undertow/util/Methods.java+5 −1 modified@@ -22,6 +22,8 @@ import java.util.HashMap; import java.util.Map; +import io.undertow.server.Connectors; + /** * NOTE: If you add a new method here you must also add it to {@link io.undertow.server.protocol.http.HttpRequestParser} * @@ -138,7 +140,9 @@ private static void putString(Map<String, HttpString> methods, HttpString option public static HttpString fromString(String method) { HttpString res = METHODS.get(method); if(res == null) { - return new HttpString(method); + HttpString httpString = new HttpString(method); + Connectors.verifyToken(httpString); + return httpString; } return res; }
core/src/test/java/io/undertow/server/handlers/form/FormDataParserTestCase.java+13 −3 modified@@ -22,8 +22,10 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.HashMap; import java.util.Iterator; import java.util.List; +import java.util.Map; import io.undertow.server.HttpHandler; import io.undertow.server.HttpServerExchange; @@ -35,6 +37,7 @@ import io.undertow.testutils.TestHttpClient; import io.undertow.util.StatusCodes; import junit.textui.TestRunner; +import org.apache.http.Header; import org.apache.http.HttpResponse; import org.apache.http.NameValuePair; import org.apache.http.client.entity.UrlEncodedFormEntity; @@ -77,7 +80,7 @@ public void handleRequest(final HttpServerExchange exchange) throws Exception { while (it.hasNext()) { String fd = it.next(); for (FormData.FormValue val : data.get(fd)) { - exchange.getResponseHeaders().add(new HttpString(fd), val.getValue()); + exchange.getResponseHeaders().add(new HttpString("res"), fd + ":" + val.getValue()); } } } @@ -100,7 +103,7 @@ public void handleRequest(final HttpServerExchange exchange) throws Exception { while (it.hasNext()) { String fd = it.next(); for (FormData.FormValue val : data.get(fd)) { - exchange.getResponseHeaders().add(new HttpString(fd), val.getValue()); + exchange.getResponseHeaders().add(new HttpString("res"), fd + ":" + val.getValue()); } } } catch (IOException e) { @@ -144,8 +147,15 @@ private void runTest(final NameValuePair... pairs) throws Exception { } private void checkResult(final List<NameValuePair> data, final HttpResponse result) { + Map<String, String> res = new HashMap<>(); + for(Header d : result.getHeaders("res")) { + String[] split = d.getValue().split(":"); + res.put(split[0], split.length == 1 ? "" : split[1]); + } + + for (NameValuePair vp : data) { - Assert.assertEquals(vp.getValue() == null ? "" : vp.getValue(), result.getHeaders(vp.getName())[0].getValue()); + Assert.assertEquals(vp.getValue() == null ? "" : vp.getValue(), res.get(vp.getName())); } }
core/src/test/java/io/undertow/server/InvalidHtpRequestTestCase.java+134 −0 added@@ -0,0 +1,134 @@ +/* + * JBoss, Home of Professional Open Source. + * Copyright 2014 Red Hat, Inc., and individual contributors + * as indicated by the @author tags. + * + * Licensed 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 io.undertow.server; + +import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; + +import org.apache.http.HttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpRequestBase; +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import io.undertow.server.handlers.ResponseCodeHandler; +import io.undertow.testutils.DefaultServer; +import io.undertow.testutils.HttpOneOnly; +import io.undertow.testutils.ProxyIgnore; +import io.undertow.testutils.TestHttpClient; +import io.undertow.util.Headers; +import io.undertow.util.StatusCodes; + +/** + * @author Stuart Douglas + */ +@RunWith(DefaultServer.class) +@ProxyIgnore +@HttpOneOnly +public class InvalidHtpRequestTestCase { + + @BeforeClass + public static void setup() { + DefaultServer.setRootHandler(ResponseCodeHandler.HANDLE_200); + } + + @Test + public void testInvalidCharacterInMethod() throws IOException { + final TestHttpClient client = new TestHttpClient(); + try { + HttpRequestBase method = new HttpRequestBase() { + + @Override + public String getMethod() { + return "GET;POST"; + } + + @Override + public URI getURI() { + try { + return new URI(DefaultServer.getDefaultServerURL()); + } catch (URISyntaxException e) { + throw new RuntimeException(e); + } + } + }; + HttpResponse result = client.execute(method); + Assert.assertEquals(StatusCodes.BAD_REQUEST, result.getStatusLine().getStatusCode()); + } finally { + client.getConnectionManager().shutdown(); + } + } + + + @Test + public void testInvalidCharacterInHeader() throws IOException { + final TestHttpClient client = new TestHttpClient(); + try { + HttpRequestBase method = new HttpGet(DefaultServer.getDefaultServerURL()); + method.addHeader("fake;header", "value"); + HttpResponse result = client.execute(method); + Assert.assertEquals(StatusCodes.BAD_REQUEST, result.getStatusLine().getStatusCode()); + } finally { + client.getConnectionManager().shutdown(); + } + } + + @Test + public void testMultipleContentLengths() throws IOException { + final TestHttpClient client = new TestHttpClient(); + try { + HttpRequestBase method = new HttpGet(DefaultServer.getDefaultServerURL()); + method.addHeader(Headers.CONTENT_LENGTH_STRING, "0"); + method.addHeader(Headers.CONTENT_LENGTH_STRING, "10"); + HttpResponse result = client.execute(method); + Assert.assertEquals(StatusCodes.BAD_REQUEST, result.getStatusLine().getStatusCode()); + } finally { + client.getConnectionManager().shutdown(); + } + } + @Test + public void testContentLengthAndTransferEncoding() throws IOException { + final TestHttpClient client = new TestHttpClient(); + try { + HttpRequestBase method = new HttpGet(DefaultServer.getDefaultServerURL()); + method.addHeader(Headers.CONTENT_LENGTH_STRING, "0"); + method.addHeader(Headers.TRANSFER_ENCODING_STRING, "chunked"); + HttpResponse result = client.execute(method); + Assert.assertEquals(StatusCodes.BAD_REQUEST, result.getStatusLine().getStatusCode()); + } finally { + client.getConnectionManager().shutdown(); + } + } + + @Test + public void testMultipleTransferEncoding() throws IOException { + final TestHttpClient client = new TestHttpClient(); + try { + HttpRequestBase method = new HttpGet(DefaultServer.getDefaultServerURL()); + method.addHeader(Headers.TRANSFER_ENCODING_STRING, "chunked"); + method.addHeader(Headers.TRANSFER_ENCODING_STRING, "gzip, chunked"); + HttpResponse result = client.execute(method); + Assert.assertEquals(StatusCodes.BAD_REQUEST, result.getStatusLine().getStatusCode()); + } finally { + client.getConnectionManager().shutdown(); + } + } +}
core/src/test/java/io/undertow/server/protocol/http/SimpleParserTestCase.java+7 −9 modified@@ -167,16 +167,16 @@ public void testLineFeedsLineEnds() throws BadRequestException { runTest(in); } - @Test + @Test(expected = BadRequestException.class) public void testTabWhitespace() throws BadRequestException { byte[] in = "GET\t/somepath\tHTTP/1.1\nHost: \t www.somehost.net\nOtherHeader:\tsome\n \t value\n\r\n".getBytes(); runTest(in); } @Test - public void testCanonicalPath() throws BadRequestException { - byte[] in = "GET\thttp://www.somehost.net/somepath\tHTTP/1.1\nHost: \t www.somehost.net\nOtherHeader:\tsome\n \t value\n\r\n".getBytes(); + public void testCanonicalPath() throws BadRequestException { + byte[] in = "GET http://www.somehost.net/somepath HTTP/1.1\nHost: \t www.somehost.net\nOtherHeader:\tsome\n \t value\n\r\n".getBytes(); final ParseState context = new ParseState(5); HttpServerExchange result = new HttpServerExchange(null); HttpRequestParser.instance(OptionMap.EMPTY).handle(ByteBuffer.wrap(in), context, result); @@ -186,7 +186,7 @@ public void testCanonicalPath() throws BadRequestException { @Test public void testNoHeaders() throws BadRequestException { - byte[] in = "GET\t/aa\tHTTP/1.1\n\n\n".getBytes(); + byte[] in = "GET /aa HTTP/1.1\n\n\n".getBytes(); final ParseState context = new ParseState(0); HttpServerExchange result = new HttpServerExchange(null); @@ -215,7 +215,7 @@ public void testQueryParams() throws BadRequestException { @Test public void testSameHttpStringReturned() throws BadRequestException { - byte[] in = "GET\thttp://www.somehost.net/somepath\tHTTP/1.1\nHost: \t www.somehost.net\nAccept-Charset:\tsome\n \t value\n\r\n".getBytes(); + byte[] in = "GET http://www.somehost.net/somepath HTTP/1.1\nHost: \t www.somehost.net\nAccept-Charset:\tsome\n \t value\n\r\n".getBytes(); final ParseState context1 = new ParseState(10); HttpServerExchange result1 = new HttpServerExchange(null); @@ -259,16 +259,14 @@ public void testEmptyQueryParams() throws BadRequestException { Assert.assertEquals("666", result.getQueryParameters().get("777").getFirst()); Assert.assertEquals("44", result.getQueryParameters().get(";?").getFirst()); } - @Test + + @Test(expected = BadRequestException.class) public void testNonEncodedAsciiCharacters() throws UnsupportedEncodingException, BadRequestException { byte[] in = "GET /bÃ¥r HTTP/1.1\r\n\r\n".getBytes("ISO-8859-1"); final ParseState context = new ParseState(10); HttpServerExchange result = new HttpServerExchange(null); HttpRequestParser.instance(OptionMap.EMPTY).handle(ByteBuffer.wrap(in), context, result); - Assert.assertSame(Methods.GET, result.getRequestMethod()); - Assert.assertEquals("/bår", result.getRequestPath()); - Assert.assertEquals("/bÃ¥r", result.getRequestURI()); //not decoded } private void runTest(final byte[] in) throws BadRequestException {
parser-generator/src/main/java/io/undertow/annotationprocessor/RequestParserGenerator.java+5 −0 modified@@ -32,6 +32,7 @@ public class RequestParserGenerator extends AbstractParserGenerator { public static final String PARSE_STATE_CLASS = "io.undertow.server.protocol.http.ParseState"; public static final String HTTP_EXCHANGE_CLASS = "io.undertow.server.HttpServerExchange"; public static final String HTTP_EXCHANGE_DESCRIPTOR = "Lio/undertow/server/HttpServerExchange;"; + private static final String CONNECTORS_CLASS = "io.undertow.server.Connectors"; //parsing states @@ -66,6 +67,8 @@ public boolean isHeader() { public void handleOtherToken(final CodeAttribute c) { c.aload(PARSE_STATE_VAR); c.swap(); + c.dup(); + c.invokestatic(CONNECTORS_CLASS, "verifyToken", "(" + HTTP_STRING_DESCRIPTOR + ")V"); c.putfield(parseStateClass, "nextHeader", HTTP_STRING_DESCRIPTOR); } @@ -150,6 +153,8 @@ public void handleStateMachineMatchedToken(final CodeAttribute c) { public void handleOtherToken(final CodeAttribute c) { c.aload(HTTP_RESULT); c.swap(); + c.dup(); + c.invokestatic(CONNECTORS_CLASS, "verifyToken", "(" + HTTP_STRING_DESCRIPTOR + ")V"); c.invokevirtual(resultClass, "setRequestMethod", "(" + HTTP_STRING_DESCRIPTOR + ")" + HTTP_EXCHANGE_DESCRIPTOR); c.pop(); }
servlet/src/test/java/io/undertow/servlet/test/path/PathFilter.java+1 −1 modified@@ -43,7 +43,7 @@ public void init(final FilterConfig filterConfig) throws ServletException { @Override public void doFilter(final ServletRequest request, final ServletResponse response, final FilterChain chain) throws IOException, ServletException { HttpServletResponse resp = (HttpServletResponse) response; - resp.addHeader("filter" + filterConfig.getFilterName(), filterConfig.getFilterName()); + resp.addHeader("filter" + filterConfig.getFilterName().replace("/", "-"), filterConfig.getFilterName()); chain.doFilter(request, response); }
1e72647818c9JBEAP-9909 Prevent HTTP smuggling attacks by making sure messages do not contain invalid headers.
14 files changed · +299 −15
core/src/main/java/io/undertow/protocols/http2/Http2HeaderBlockParser.java+4 −0 modified@@ -28,6 +28,7 @@ import io.undertow.UndertowLogger; import io.undertow.UndertowMessages; +import io.undertow.server.Connectors; import io.undertow.util.HeaderMap; import io.undertow.util.Headers; import io.undertow.util.HttpString; @@ -177,6 +178,9 @@ public void emitHeader(HttpString name, String value, boolean neverIndex) throws if(c>= 'A' && c <= 'Z') { invalid = true; UndertowLogger.REQUEST_LOGGER.debugf("Malformed request, header %s contains uppercase characters", name); + } else if(c != ':' && !Connectors.isValidTokenCharacter(c)) { + invalid = true; + UndertowLogger.REQUEST_LOGGER.debugf("Malformed request, header %s contains invalid token character", name); } }
core/src/main/java/io/undertow/server/Connectors.java+76 −0 modified@@ -19,10 +19,14 @@ package io.undertow.server; import io.undertow.UndertowLogger; +import io.undertow.UndertowMessages; import io.undertow.UndertowOptions; import io.undertow.server.handlers.Cookie; import io.undertow.util.DateUtils; +import io.undertow.util.HeaderMap; +import io.undertow.util.HeaderValues; import io.undertow.util.Headers; +import io.undertow.util.HttpString; import io.undertow.util.ParameterLimitException; import io.undertow.util.StatusCodes; import io.undertow.util.URLUtils; @@ -46,7 +50,40 @@ */ public class Connectors { + private static final boolean[] ALLOWED_TOKEN_CHARACTERS = new boolean[256]; + static { + for(int i = 0; i < ALLOWED_TOKEN_CHARACTERS.length; ++i) { + if((i >='0' && i <= '9') || + (i >='a' && i <= 'z') || + (i >='A' && i <= 'Z')) { + ALLOWED_TOKEN_CHARACTERS[i] = true; + } else { + switch (i) { + case '!': + case '#': + case '$': + case '%': + case '&': + case '\'': + case '*': + case '+': + case '-': + case '.': + case '^': + case '_': + case '`': + case '|': + case '~': { + ALLOWED_TOKEN_CHARACTERS[i] = true; + break; + } + default: + ALLOWED_TOKEN_CHARACTERS[i] = false; + } + } + } + } /** * Flattens the exchange cookie map into the response header map. This should be called by a * connector just before the response is started. @@ -379,4 +416,43 @@ public static void updateResponseBytesSent(HttpServerExchange exchange, long byt public static ConduitStreamSinkChannel getConduitSinkChannel(HttpServerExchange exchange) { return exchange.getConnection().getSinkChannel(); } + + /** + * Verifies that the contents of the HttpString are a valid token according to rfc7230. + * @param header The header to verify + */ + public static void verifyToken(HttpString header) { + int length = header.length(); + for(int i = 0; i < length; ++i) { + byte c = header.byteAt(i); + if(!ALLOWED_TOKEN_CHARACTERS[c]) { + throw UndertowMessages.MESSAGES.invalidToken(c); + } + } + } + + /** + * Returns true if the token character is valid according to rfc7230 + */ + public static boolean isValidTokenCharacter(byte c) { + return ALLOWED_TOKEN_CHARACTERS[c]; + } + + + /** + * Verifies that the provided request headers are valid according to rfc7230. In particular: + * - At most one content-length or transfer encoding + */ + public static boolean areRequestHeadersValid(HeaderMap headers) { + HeaderValues te = headers.get(Headers.TRANSFER_ENCODING); + HeaderValues cl = headers.get(Headers.CONTENT_LENGTH); + if(te != null && cl != null) { + return false; + } else if(te != null && te.size() > 1) { + return false; + } else if(cl != null && cl.size() > 1) { + return false; + } + return true; + } }
core/src/main/java/io/undertow/server/protocol/ajp/AjpReadListener.java+4 −0 modified@@ -238,6 +238,10 @@ public void handleEvent(AjpServerResponseConduit channel) { if(connectorStatistics != null) { connectorStatistics.setup(httpServerExchange); } + if(!Connectors.areRequestHeadersValid(httpServerExchange.getRequestHeaders())) { + oldState.badRequest = true; + UndertowLogger.REQUEST_IO_LOGGER.debugf("Invalid AJP request from %s, request contained invalid headers", connection.getPeerAddress()); + } if(oldState.badRequest) { httpServerExchange.setStatusCode(StatusCodes.BAD_REQUEST);
core/src/main/java/io/undertow/server/protocol/ajp/AjpRequestParser.java+5 −1 modified@@ -55,6 +55,7 @@ import io.undertow.UndertowLogger; import io.undertow.UndertowMessages; import io.undertow.security.impl.ExternalAuthenticationMechanism; +import io.undertow.server.Connectors; import io.undertow.server.HttpServerExchange; import io.undertow.util.Headers; import io.undertow.util.HttpString; @@ -345,6 +346,7 @@ public void parse(final ByteBuffer buf, final AjpRequestParseState state, final state.currentHeader = result.header; } else { state.currentHeader = HttpString.tryFromString(result.value); + Connectors.verifyToken(state.currentHeader); } } StringHolder result = parseString(buf, state, StringType.OTHER); @@ -423,7 +425,9 @@ public void parse(final ByteBuffer buf, final AjpRequestParseState state, final } else if (state.currentAttribute.equals(AUTH_TYPE)) { exchange.putAttachment(ExternalAuthenticationMechanism.EXTERNAL_AUTHENTICATION_TYPE, result); } else if (state.currentAttribute.equals(STORED_METHOD)) { - exchange.setRequestMethod(new HttpString(result)); + HttpString requestMethod = new HttpString(result); + Connectors.verifyToken(requestMethod); + exchange.setRequestMethod(requestMethod); } else if (state.currentAttribute.equals(AJP_REMOTE_PORT)) { state.remotePort = Integer.parseInt(result); } else if (state.currentAttribute.equals(SSL_SESSION)) {
core/src/main/java/io/undertow/server/protocol/http2/Http2ReceiveListener.java+5 −0 modified@@ -145,6 +145,11 @@ private void handleRequests(Http2Channel channel, Http2StreamSourceChannel frame exchange.setProtocol(Protocols.HTTP_2_0); exchange.setRequestMethod(Methods.fromString(exchange.getRequestHeaders().getFirst(METHOD))); exchange.getRequestHeaders().put(Headers.HOST, exchange.getRequestHeaders().getFirst(AUTHORITY)); + if(!Connectors.areRequestHeadersValid(exchange.getRequestHeaders())) { + UndertowLogger.REQUEST_IO_LOGGER.debugf("Invalid headers in HTTP/2 request, closing connection. Remote peer %s", connection.getPeerAddress()); + channel.sendGoAway(Http2Channel.ERROR_PROTOCOL_ERROR); + return; + } final String path = exchange.getRequestHeaders().getFirst(PATH); if(path == null || path.isEmpty()) {
core/src/main/java/io/undertow/server/protocol/http/HttpReadListener.java+4 −0 modified@@ -241,6 +241,10 @@ public void handleEventWithNoRunningRequest(final ConduitStreamSourceChannel cha return; } } + if(!Connectors.areRequestHeadersValid(httpServerExchange.getRequestHeaders())) { + sendBadRequestAndClose(connection.getChannel(), UndertowMessages.MESSAGES.invalidHeaders()); + return; + } Connectors.executeRootHandler(connection.getRootHandler(), httpServerExchange); } catch (Exception e) { sendBadRequestAndClose(connection.getChannel(), e);
core/src/main/java/io/undertow/server/protocol/http/HttpRequestParser.java+27 −0 modified@@ -167,13 +167,37 @@ public abstract class HttpRequestParser { private final String charset; private final int maxCachedHeaderSize; + private static final boolean[] ALLOWED_TARGET_CHARACTER = new boolean[256]; + static { try { HTTP = "HTTP/1.".getBytes("ASCII"); HTTP_LENGTH = HTTP.length; } catch (UnsupportedEncodingException e) { throw new RuntimeException(e); } + for(int i = 0; i < 256; ++i) { + if(i < 32 || i > 126) { + ALLOWED_TARGET_CHARACTER[i] = false; + } else { + switch ((char)i) { + case '\"': + case '#': + case '<': + case '>': + case '\\': + case '^': + case '`': + case '{': + case '|': + case '}': + ALLOWED_TARGET_CHARACTER[i] = false; + break; + default: + ALLOWED_TARGET_CHARACTER[i] = true; + } + } + } } public HttpRequestParser(OptionMap options) { @@ -348,6 +372,9 @@ final void handlePath(ByteBuffer buffer, ParseState state, HttpServerExchange ex while (buffer.hasRemaining()) { char next = (char) (buffer.get() & 0xFF); + if(!ALLOWED_TARGET_CHARACTER[next]) { + throw new BadRequestException(UndertowMessages.MESSAGES.invalidCharacterInRequestTarget(next)); + } if (next == ' ' || next == '\t') { if (stringBuilder.length() != 0) { final String path = stringBuilder.toString();
core/src/main/java/io/undertow/UndertowMessages.java+9 −0 modified@@ -516,4 +516,13 @@ public interface UndertowMessages { @Message(id = 162, value = "Same-site attribute %s is invalid. It must be Strict or Lax") IllegalArgumentException invalidSameSiteMode(String mode); + + @Message(id = 163, value = "Invalid token %s") + IllegalArgumentException invalidToken(byte c); + + @Message(id = 164, value = "Request contained invalid headers") + IllegalArgumentException invalidHeaders(); + + @Message(id = 165, value = "Invalid character %s in request-target") + String invalidCharacterInRequestTarget(char next); }
core/src/main/java/io/undertow/util/Methods.java+5 −1 modified@@ -22,6 +22,8 @@ import java.util.HashMap; import java.util.Map; +import io.undertow.server.Connectors; + /** * NOTE: If you add a new method here you must also add it to {@link io.undertow.server.protocol.http.HttpRequestParser} * @@ -138,7 +140,9 @@ private static void putString(Map<String, HttpString> methods, HttpString option public static HttpString fromString(String method) { HttpString res = METHODS.get(method); if(res == null) { - return new HttpString(method); + HttpString httpString = new HttpString(method); + Connectors.verifyToken(httpString); + return httpString; } return res; }
core/src/test/java/io/undertow/server/handlers/form/FormDataParserTestCase.java+13 −3 modified@@ -22,8 +22,10 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.HashMap; import java.util.Iterator; import java.util.List; +import java.util.Map; import io.undertow.server.HttpHandler; import io.undertow.server.HttpServerExchange; @@ -35,6 +37,7 @@ import io.undertow.testutils.TestHttpClient; import io.undertow.util.StatusCodes; import junit.textui.TestRunner; +import org.apache.http.Header; import org.apache.http.HttpResponse; import org.apache.http.NameValuePair; import org.apache.http.client.entity.UrlEncodedFormEntity; @@ -77,7 +80,7 @@ public void handleRequest(final HttpServerExchange exchange) throws Exception { while (it.hasNext()) { String fd = it.next(); for (FormData.FormValue val : data.get(fd)) { - exchange.getResponseHeaders().add(new HttpString(fd), val.getValue()); + exchange.getResponseHeaders().add(new HttpString("res"), fd + ":" + val.getValue()); } } } @@ -100,7 +103,7 @@ public void handleRequest(final HttpServerExchange exchange) throws Exception { while (it.hasNext()) { String fd = it.next(); for (FormData.FormValue val : data.get(fd)) { - exchange.getResponseHeaders().add(new HttpString(fd), val.getValue()); + exchange.getResponseHeaders().add(new HttpString("res"), fd + ":" + val.getValue()); } } } catch (IOException e) { @@ -144,8 +147,15 @@ private void runTest(final NameValuePair... pairs) throws Exception { } private void checkResult(final List<NameValuePair> data, final HttpResponse result) { + Map<String, String> res = new HashMap<>(); + for(Header d : result.getHeaders("res")) { + String[] split = d.getValue().split(":"); + res.put(split[0], split.length == 1 ? "" : split[1]); + } + + for (NameValuePair vp : data) { - Assert.assertEquals(vp.getValue() == null ? "" : vp.getValue(), result.getHeaders(vp.getName())[0].getValue()); + Assert.assertEquals(vp.getValue() == null ? "" : vp.getValue(), res.get(vp.getName())); } }
core/src/test/java/io/undertow/server/InvalidHtpRequestTestCase.java+134 −0 added@@ -0,0 +1,134 @@ +/* + * JBoss, Home of Professional Open Source. + * Copyright 2014 Red Hat, Inc., and individual contributors + * as indicated by the @author tags. + * + * Licensed 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 io.undertow.server; + +import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; + +import org.apache.http.HttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpRequestBase; +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import io.undertow.server.handlers.ResponseCodeHandler; +import io.undertow.testutils.DefaultServer; +import io.undertow.testutils.HttpOneOnly; +import io.undertow.testutils.ProxyIgnore; +import io.undertow.testutils.TestHttpClient; +import io.undertow.util.Headers; +import io.undertow.util.StatusCodes; + +/** + * @author Stuart Douglas + */ +@RunWith(DefaultServer.class) +@ProxyIgnore +@HttpOneOnly +public class InvalidHtpRequestTestCase { + + @BeforeClass + public static void setup() { + DefaultServer.setRootHandler(ResponseCodeHandler.HANDLE_200); + } + + @Test + public void testInvalidCharacterInMethod() throws IOException { + final TestHttpClient client = new TestHttpClient(); + try { + HttpRequestBase method = new HttpRequestBase() { + + @Override + public String getMethod() { + return "GET;POST"; + } + + @Override + public URI getURI() { + try { + return new URI(DefaultServer.getDefaultServerURL()); + } catch (URISyntaxException e) { + throw new RuntimeException(e); + } + } + }; + HttpResponse result = client.execute(method); + Assert.assertEquals(StatusCodes.BAD_REQUEST, result.getStatusLine().getStatusCode()); + } finally { + client.getConnectionManager().shutdown(); + } + } + + + @Test + public void testInvalidCharacterInHeader() throws IOException { + final TestHttpClient client = new TestHttpClient(); + try { + HttpRequestBase method = new HttpGet(DefaultServer.getDefaultServerURL()); + method.addHeader("fake;header", "value"); + HttpResponse result = client.execute(method); + Assert.assertEquals(StatusCodes.BAD_REQUEST, result.getStatusLine().getStatusCode()); + } finally { + client.getConnectionManager().shutdown(); + } + } + + @Test + public void testMultipleContentLengths() throws IOException { + final TestHttpClient client = new TestHttpClient(); + try { + HttpRequestBase method = new HttpGet(DefaultServer.getDefaultServerURL()); + method.addHeader(Headers.CONTENT_LENGTH_STRING, "0"); + method.addHeader(Headers.CONTENT_LENGTH_STRING, "10"); + HttpResponse result = client.execute(method); + Assert.assertEquals(StatusCodes.BAD_REQUEST, result.getStatusLine().getStatusCode()); + } finally { + client.getConnectionManager().shutdown(); + } + } + @Test + public void testContentLengthAndTransferEncoding() throws IOException { + final TestHttpClient client = new TestHttpClient(); + try { + HttpRequestBase method = new HttpGet(DefaultServer.getDefaultServerURL()); + method.addHeader(Headers.CONTENT_LENGTH_STRING, "0"); + method.addHeader(Headers.TRANSFER_ENCODING_STRING, "chunked"); + HttpResponse result = client.execute(method); + Assert.assertEquals(StatusCodes.BAD_REQUEST, result.getStatusLine().getStatusCode()); + } finally { + client.getConnectionManager().shutdown(); + } + } + + @Test + public void testMultipleTransferEncoding() throws IOException { + final TestHttpClient client = new TestHttpClient(); + try { + HttpRequestBase method = new HttpGet(DefaultServer.getDefaultServerURL()); + method.addHeader(Headers.TRANSFER_ENCODING_STRING, "chunked"); + method.addHeader(Headers.TRANSFER_ENCODING_STRING, "gzip, chunked"); + HttpResponse result = client.execute(method); + Assert.assertEquals(StatusCodes.BAD_REQUEST, result.getStatusLine().getStatusCode()); + } finally { + client.getConnectionManager().shutdown(); + } + } +}
core/src/test/java/io/undertow/server/protocol/http/SimpleParserTestCase.java+7 −9 modified@@ -167,16 +167,16 @@ public void testLineFeedsLineEnds() throws BadRequestException { runTest(in); } - @Test + @Test(expected = BadRequestException.class) public void testTabWhitespace() throws BadRequestException { byte[] in = "GET\t/somepath\tHTTP/1.1\nHost: \t www.somehost.net\nOtherHeader:\tsome\n \t value\n\r\n".getBytes(); runTest(in); } @Test - public void testCanonicalPath() throws BadRequestException { - byte[] in = "GET\thttp://www.somehost.net/somepath\tHTTP/1.1\nHost: \t www.somehost.net\nOtherHeader:\tsome\n \t value\n\r\n".getBytes(); + public void testCanonicalPath() throws BadRequestException { + byte[] in = "GET http://www.somehost.net/somepath HTTP/1.1\nHost: \t www.somehost.net\nOtherHeader:\tsome\n \t value\n\r\n".getBytes(); final ParseState context = new ParseState(5); HttpServerExchange result = new HttpServerExchange(null); HttpRequestParser.instance(OptionMap.EMPTY).handle(ByteBuffer.wrap(in), context, result); @@ -186,7 +186,7 @@ public void testCanonicalPath() throws BadRequestException { @Test public void testNoHeaders() throws BadRequestException { - byte[] in = "GET\t/aa\tHTTP/1.1\n\n\n".getBytes(); + byte[] in = "GET /aa HTTP/1.1\n\n\n".getBytes(); final ParseState context = new ParseState(0); HttpServerExchange result = new HttpServerExchange(null); @@ -215,7 +215,7 @@ public void testQueryParams() throws BadRequestException { @Test public void testSameHttpStringReturned() throws BadRequestException { - byte[] in = "GET\thttp://www.somehost.net/somepath\tHTTP/1.1\nHost: \t www.somehost.net\nAccept-Charset:\tsome\n \t value\n\r\n".getBytes(); + byte[] in = "GET http://www.somehost.net/somepath HTTP/1.1\nHost: \t www.somehost.net\nAccept-Charset:\tsome\n \t value\n\r\n".getBytes(); final ParseState context1 = new ParseState(10); HttpServerExchange result1 = new HttpServerExchange(null); @@ -259,16 +259,14 @@ public void testEmptyQueryParams() throws BadRequestException { Assert.assertEquals("666", result.getQueryParameters().get("777").getFirst()); Assert.assertEquals("44", result.getQueryParameters().get(";?").getFirst()); } - @Test + + @Test(expected = BadRequestException.class) public void testNonEncodedAsciiCharacters() throws UnsupportedEncodingException, BadRequestException { byte[] in = "GET /bÃ¥r HTTP/1.1\r\n\r\n".getBytes("ISO-8859-1"); final ParseState context = new ParseState(10); HttpServerExchange result = new HttpServerExchange(null); HttpRequestParser.instance(OptionMap.EMPTY).handle(ByteBuffer.wrap(in), context, result); - Assert.assertSame(Methods.GET, result.getRequestMethod()); - Assert.assertEquals("/bår", result.getRequestPath()); - Assert.assertEquals("/bÃ¥r", result.getRequestURI()); //not decoded } private void runTest(final byte[] in) throws BadRequestException {
parser-generator/src/main/java/io/undertow/annotationprocessor/RequestParserGenerator.java+5 −0 modified@@ -32,6 +32,7 @@ public class RequestParserGenerator extends AbstractParserGenerator { public static final String PARSE_STATE_CLASS = "io.undertow.server.protocol.http.ParseState"; public static final String HTTP_EXCHANGE_CLASS = "io.undertow.server.HttpServerExchange"; public static final String HTTP_EXCHANGE_DESCRIPTOR = "Lio/undertow/server/HttpServerExchange;"; + private static final String CONNECTORS_CLASS = "io.undertow.server.Connectors"; //parsing states @@ -66,6 +67,8 @@ public boolean isHeader() { public void handleOtherToken(final CodeAttribute c) { c.aload(PARSE_STATE_VAR); c.swap(); + c.dup(); + c.invokestatic(CONNECTORS_CLASS, "verifyToken", "(" + HTTP_STRING_DESCRIPTOR + ")V"); c.putfield(parseStateClass, "nextHeader", HTTP_STRING_DESCRIPTOR); } @@ -150,6 +153,8 @@ public void handleStateMachineMatchedToken(final CodeAttribute c) { public void handleOtherToken(final CodeAttribute c) { c.aload(HTTP_RESULT); c.swap(); + c.dup(); + c.invokestatic(CONNECTORS_CLASS, "verifyToken", "(" + HTTP_STRING_DESCRIPTOR + ")V"); c.invokevirtual(resultClass, "setRequestMethod", "(" + HTTP_STRING_DESCRIPTOR + ")" + HTTP_EXCHANGE_DESCRIPTOR); c.pop(); }
servlet/src/test/java/io/undertow/servlet/test/path/PathFilter.java+1 −1 modified@@ -43,7 +43,7 @@ public void init(final FilterConfig filterConfig) throws ServletException { @Override public void doFilter(final ServletRequest request, final ServletResponse response, final FilterChain chain) throws IOException, ServletException { HttpServletResponse resp = (HttpServletResponse) response; - resp.addHeader("filter" + filterConfig.getFilterName(), filterConfig.getFilterName()); + resp.addHeader("filter" + filterConfig.getFilterName().replace("/", "-"), filterConfig.getFilterName()); chain.doFilter(request, response); }
691440ee5825JBEAP-9909 Prevent HTTP smuggling attacks by making sure messages do not contain invalid headers.
14 files changed · +300 −15
core/src/main/java/io/undertow/protocols/http2/Http2HeaderBlockParser.java+4 −0 modified@@ -28,6 +28,7 @@ import io.undertow.UndertowLogger; import io.undertow.UndertowMessages; +import io.undertow.server.Connectors; import io.undertow.util.HeaderMap; import io.undertow.util.Headers; import io.undertow.util.HttpString; @@ -177,6 +178,9 @@ public void emitHeader(HttpString name, String value, boolean neverIndex) throws if(c>= 'A' && c <= 'Z') { invalid = true; UndertowLogger.REQUEST_LOGGER.debugf("Malformed request, header %s contains uppercase characters", name); + } else if(c != ':' && !Connectors.isValidTokenCharacter(c)) { + invalid = true; + UndertowLogger.REQUEST_LOGGER.debugf("Malformed request, header %s contains invalid token character", name); } }
core/src/main/java/io/undertow/server/Connectors.java+76 −0 modified@@ -19,10 +19,14 @@ package io.undertow.server; import io.undertow.UndertowLogger; +import io.undertow.UndertowMessages; import io.undertow.UndertowOptions; import io.undertow.server.handlers.Cookie; import io.undertow.util.DateUtils; +import io.undertow.util.HeaderMap; +import io.undertow.util.HeaderValues; import io.undertow.util.Headers; +import io.undertow.util.HttpString; import io.undertow.util.ParameterLimitException; import io.undertow.util.StatusCodes; import io.undertow.util.URLUtils; @@ -44,7 +48,40 @@ */ public class Connectors { + private static final boolean[] ALLOWED_TOKEN_CHARACTERS = new boolean[256]; + static { + for(int i = 0; i < ALLOWED_TOKEN_CHARACTERS.length; ++i) { + if((i >='0' && i <= '9') || + (i >='a' && i <= 'z') || + (i >='A' && i <= 'Z')) { + ALLOWED_TOKEN_CHARACTERS[i] = true; + } else { + switch (i) { + case '!': + case '#': + case '$': + case '%': + case '&': + case '\'': + case '*': + case '+': + case '-': + case '.': + case '^': + case '_': + case '`': + case '|': + case '~': { + ALLOWED_TOKEN_CHARACTERS[i] = true; + break; + } + default: + ALLOWED_TOKEN_CHARACTERS[i] = false; + } + } + } + } /** * Flattens the exchange cookie map into the response header map. This should be called by a * connector just before the response is started. @@ -350,4 +387,43 @@ public static void updateResponseBytesSent(HttpServerExchange exchange, long byt public static ConduitStreamSinkChannel getConduitSinkChannel(HttpServerExchange exchange) { return exchange.getConnection().getSinkChannel(); } + + /** + * Verifies that the contents of the HttpString are a valid token according to rfc7230. + * @param header The header to verify + */ + public static void verifyToken(HttpString header) { + int length = header.length(); + for(int i = 0; i < length; ++i) { + byte c = header.byteAt(i); + if(!ALLOWED_TOKEN_CHARACTERS[c]) { + throw UndertowMessages.MESSAGES.invalidToken(c); + } + } + } + + /** + * Returns true if the token character is valid according to rfc7230 + */ + public static boolean isValidTokenCharacter(byte c) { + return ALLOWED_TOKEN_CHARACTERS[c]; + } + + + /** + * Verifies that the provided request headers are valid according to rfc7230. In particular: + * - At most one content-length or transfer encoding + */ + public static boolean areRequestHeadersValid(HeaderMap headers) { + HeaderValues te = headers.get(Headers.TRANSFER_ENCODING); + HeaderValues cl = headers.get(Headers.CONTENT_LENGTH); + if(te != null && cl != null) { + return false; + } else if(te != null && te.size() > 1) { + return false; + } else if(cl != null && cl.size() > 1) { + return false; + } + return true; + } }
core/src/main/java/io/undertow/server/protocol/ajp/AjpReadListener.java+4 −0 modified@@ -238,6 +238,10 @@ public void handleEvent(AjpServerResponseConduit channel) { if(connectorStatistics != null) { connectorStatistics.setup(httpServerExchange); } + if(!Connectors.areRequestHeadersValid(httpServerExchange.getRequestHeaders())) { + oldState.badRequest = true; + UndertowLogger.REQUEST_IO_LOGGER.debugf("Invalid AJP request from %s, request contained invalid headers", connection.getPeerAddress()); + } if(oldState.badRequest) { httpServerExchange.setStatusCode(StatusCodes.BAD_REQUEST);
core/src/main/java/io/undertow/server/protocol/ajp/AjpRequestParser.java+5 −1 modified@@ -55,6 +55,7 @@ import io.undertow.UndertowLogger; import io.undertow.UndertowMessages; import io.undertow.security.impl.ExternalAuthenticationMechanism; +import io.undertow.server.Connectors; import io.undertow.server.HttpServerExchange; import io.undertow.util.Headers; import io.undertow.util.HttpString; @@ -345,6 +346,7 @@ public void parse(final ByteBuffer buf, final AjpRequestParseState state, final state.currentHeader = result.header; } else { state.currentHeader = HttpString.tryFromString(result.value); + Connectors.verifyToken(state.currentHeader); } } StringHolder result = parseString(buf, state, StringType.OTHER); @@ -423,7 +425,9 @@ public void parse(final ByteBuffer buf, final AjpRequestParseState state, final } else if (state.currentAttribute.equals(AUTH_TYPE)) { exchange.putAttachment(ExternalAuthenticationMechanism.EXTERNAL_AUTHENTICATION_TYPE, result); } else if (state.currentAttribute.equals(STORED_METHOD)) { - exchange.setRequestMethod(new HttpString(result)); + HttpString requestMethod = new HttpString(result); + Connectors.verifyToken(requestMethod); + exchange.setRequestMethod(requestMethod); } else if (state.currentAttribute.equals(AJP_REMOTE_PORT)) { state.remotePort = Integer.parseInt(result); } else if (state.currentAttribute.equals(SSL_SESSION)) {
core/src/main/java/io/undertow/server/protocol/http2/Http2ReceiveListener.java+5 −0 modified@@ -146,6 +146,11 @@ private void handleRequests(Http2Channel channel, Http2StreamSourceChannel frame exchange.setProtocol(Protocols.HTTP_2_0); exchange.setRequestMethod(Methods.fromString(exchange.getRequestHeaders().getFirst(METHOD))); exchange.getRequestHeaders().put(Headers.HOST, exchange.getRequestHeaders().getFirst(AUTHORITY)); + if(!Connectors.areRequestHeadersValid(exchange.getRequestHeaders())) { + UndertowLogger.REQUEST_IO_LOGGER.debugf("Invalid headers in HTTP/2 request, closing connection. Remote peer %s", connection.getPeerAddress()); + channel.sendGoAway(Http2Channel.ERROR_PROTOCOL_ERROR); + return; + } final String path = exchange.getRequestHeaders().getFirst(PATH); try {
core/src/main/java/io/undertow/server/protocol/http/HttpReadListener.java+5 −0 modified@@ -231,6 +231,11 @@ public void handleEventWithNoRunningRequest(final ConduitStreamSourceChannel cha //so we just suspend every time (the overhead is likely much less than the general SSL overhead anyway) channel.suspendReads(); } + + if(!Connectors.areRequestHeadersValid(httpServerExchange.getRequestHeaders())) { + sendBadRequestAndClose(connection.getChannel(), UndertowMessages.MESSAGES.invalidHeaders()); + return; + } Connectors.executeRootHandler(connection.getRootHandler(), httpServerExchange); } catch (Exception e) { sendBadRequestAndClose(connection.getChannel(), e);
core/src/main/java/io/undertow/server/protocol/http/HttpRequestParser.java+27 −0 modified@@ -167,13 +167,37 @@ public abstract class HttpRequestParser { private final String charset; private final int maxCachedHeaderSize; + private static final boolean[] ALLOWED_TARGET_CHARACTER = new boolean[256]; + static { try { HTTP = "HTTP/1.".getBytes("ASCII"); HTTP_LENGTH = HTTP.length; } catch (UnsupportedEncodingException e) { throw new RuntimeException(e); } + for(int i = 0; i < 256; ++i) { + if(i < 32 || i > 126) { + ALLOWED_TARGET_CHARACTER[i] = false; + } else { + switch ((char)i) { + case '\"': + case '#': + case '<': + case '>': + case '\\': + case '^': + case '`': + case '{': + case '|': + case '}': + ALLOWED_TARGET_CHARACTER[i] = false; + break; + default: + ALLOWED_TARGET_CHARACTER[i] = true; + } + } + } } public HttpRequestParser(OptionMap options) { @@ -348,6 +372,9 @@ final void handlePath(ByteBuffer buffer, ParseState state, HttpServerExchange ex while (buffer.hasRemaining()) { char next = (char) (buffer.get() & 0xFF); + if(!ALLOWED_TARGET_CHARACTER[next]) { + throw new BadRequestException(UndertowMessages.MESSAGES.invalidCharacterInRequestTarget(next)); + } if (next == ' ' || next == '\t') { if (stringBuilder.length() != 0) { final String path = stringBuilder.toString();
core/src/main/java/io/undertow/UndertowMessages.java+9 −0 modified@@ -448,4 +448,13 @@ public interface UndertowMessages { @Message(id = 149, value = "HttpString is not allowed to contain newlines. value: %s") IllegalArgumentException newlineNotSupportedInHttpString(String value); + + @Message(id = 163, value = "Invalid token %s") + IllegalArgumentException invalidToken(byte c); + + @Message(id = 164, value = "Request contained invalid headers") + IllegalArgumentException invalidHeaders(); + + @Message(id = 165, value = "Invalid character %s in request-target") + String invalidCharacterInRequestTarget(char next); }
core/src/main/java/io/undertow/util/Methods.java+5 −1 modified@@ -22,6 +22,8 @@ import java.util.HashMap; import java.util.Map; +import io.undertow.server.Connectors; + /** * NOTE: If you add a new method here you must also add it to {@link io.undertow.server.protocol.http.HttpRequestParser} * @@ -135,7 +137,9 @@ private static void putString(Map<String, HttpString> methods, HttpString option public static HttpString fromString(String method) { HttpString res = METHODS.get(method); if(res == null) { - return new HttpString(method); + HttpString httpString = new HttpString(method); + Connectors.verifyToken(httpString); + return httpString; } return res; }
core/src/test/java/io/undertow/server/handlers/form/FormDataParserTestCase.java+13 −3 modified@@ -22,8 +22,10 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.HashMap; import java.util.Iterator; import java.util.List; +import java.util.Map; import io.undertow.server.HttpHandler; import io.undertow.server.HttpServerExchange; @@ -35,6 +37,7 @@ import io.undertow.testutils.TestHttpClient; import io.undertow.util.StatusCodes; import junit.textui.TestRunner; +import org.apache.http.Header; import org.apache.http.HttpResponse; import org.apache.http.NameValuePair; import org.apache.http.client.entity.UrlEncodedFormEntity; @@ -77,7 +80,7 @@ public void handleRequest(final HttpServerExchange exchange) throws Exception { while (it.hasNext()) { String fd = it.next(); for (FormData.FormValue val : data.get(fd)) { - exchange.getResponseHeaders().add(new HttpString(fd), val.getValue()); + exchange.getResponseHeaders().add(new HttpString("res"), fd + ":" + val.getValue()); } } } @@ -100,7 +103,7 @@ public void handleRequest(final HttpServerExchange exchange) throws Exception { while (it.hasNext()) { String fd = it.next(); for (FormData.FormValue val : data.get(fd)) { - exchange.getResponseHeaders().add(new HttpString(fd), val.getValue()); + exchange.getResponseHeaders().add(new HttpString("res"), fd + ":" + val.getValue()); } } } catch (IOException e) { @@ -144,8 +147,15 @@ private void runTest(final NameValuePair... pairs) throws Exception { } private void checkResult(final List<NameValuePair> data, final HttpResponse result) { + Map<String, String> res = new HashMap<>(); + for(Header d : result.getHeaders("res")) { + String[] split = d.getValue().split(":"); + res.put(split[0], split.length == 1 ? "" : split[1]); + } + + for (NameValuePair vp : data) { - Assert.assertEquals(vp.getValue() == null ? "" : vp.getValue(), result.getHeaders(vp.getName())[0].getValue()); + Assert.assertEquals(vp.getValue() == null ? "" : vp.getValue(), res.get(vp.getName())); } }
core/src/test/java/io/undertow/server/InvalidHtpRequestTestCase.java+134 −0 added@@ -0,0 +1,134 @@ +/* + * JBoss, Home of Professional Open Source. + * Copyright 2014 Red Hat, Inc., and individual contributors + * as indicated by the @author tags. + * + * Licensed 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 io.undertow.server; + +import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; + +import org.apache.http.HttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpRequestBase; +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import io.undertow.server.handlers.ResponseCodeHandler; +import io.undertow.testutils.DefaultServer; +import io.undertow.testutils.HttpOneOnly; +import io.undertow.testutils.ProxyIgnore; +import io.undertow.testutils.TestHttpClient; +import io.undertow.util.Headers; +import io.undertow.util.StatusCodes; + +/** + * @author Stuart Douglas + */ +@RunWith(DefaultServer.class) +@ProxyIgnore +@HttpOneOnly +public class InvalidHtpRequestTestCase { + + @BeforeClass + public static void setup() { + DefaultServer.setRootHandler(ResponseCodeHandler.HANDLE_200); + } + + @Test + public void testInvalidCharacterInMethod() throws IOException { + final TestHttpClient client = new TestHttpClient(); + try { + HttpRequestBase method = new HttpRequestBase() { + + @Override + public String getMethod() { + return "GET;POST"; + } + + @Override + public URI getURI() { + try { + return new URI(DefaultServer.getDefaultServerURL()); + } catch (URISyntaxException e) { + throw new RuntimeException(e); + } + } + }; + HttpResponse result = client.execute(method); + Assert.assertEquals(StatusCodes.BAD_REQUEST, result.getStatusLine().getStatusCode()); + } finally { + client.getConnectionManager().shutdown(); + } + } + + + @Test + public void testInvalidCharacterInHeader() throws IOException { + final TestHttpClient client = new TestHttpClient(); + try { + HttpRequestBase method = new HttpGet(DefaultServer.getDefaultServerURL()); + method.addHeader("fake;header", "value"); + HttpResponse result = client.execute(method); + Assert.assertEquals(StatusCodes.BAD_REQUEST, result.getStatusLine().getStatusCode()); + } finally { + client.getConnectionManager().shutdown(); + } + } + + @Test + public void testMultipleContentLengths() throws IOException { + final TestHttpClient client = new TestHttpClient(); + try { + HttpRequestBase method = new HttpGet(DefaultServer.getDefaultServerURL()); + method.addHeader(Headers.CONTENT_LENGTH_STRING, "0"); + method.addHeader(Headers.CONTENT_LENGTH_STRING, "10"); + HttpResponse result = client.execute(method); + Assert.assertEquals(StatusCodes.BAD_REQUEST, result.getStatusLine().getStatusCode()); + } finally { + client.getConnectionManager().shutdown(); + } + } + @Test + public void testContentLengthAndTransferEncoding() throws IOException { + final TestHttpClient client = new TestHttpClient(); + try { + HttpRequestBase method = new HttpGet(DefaultServer.getDefaultServerURL()); + method.addHeader(Headers.CONTENT_LENGTH_STRING, "0"); + method.addHeader(Headers.TRANSFER_ENCODING_STRING, "chunked"); + HttpResponse result = client.execute(method); + Assert.assertEquals(StatusCodes.BAD_REQUEST, result.getStatusLine().getStatusCode()); + } finally { + client.getConnectionManager().shutdown(); + } + } + + @Test + public void testMultipleTransferEncoding() throws IOException { + final TestHttpClient client = new TestHttpClient(); + try { + HttpRequestBase method = new HttpGet(DefaultServer.getDefaultServerURL()); + method.addHeader(Headers.TRANSFER_ENCODING_STRING, "chunked"); + method.addHeader(Headers.TRANSFER_ENCODING_STRING, "gzip, chunked"); + HttpResponse result = client.execute(method); + Assert.assertEquals(StatusCodes.BAD_REQUEST, result.getStatusLine().getStatusCode()); + } finally { + client.getConnectionManager().shutdown(); + } + } +}
core/src/test/java/io/undertow/server/protocol/http/SimpleParserTestCase.java+7 −9 modified@@ -164,16 +164,16 @@ public void testLineFeedsLineEnds() throws BadRequestException { runTest(in); } - @Test + @Test(expected = BadRequestException.class) public void testTabWhitespace() throws BadRequestException { byte[] in = "GET\t/somepath\tHTTP/1.1\nHost: \t www.somehost.net\nOtherHeader:\tsome\n \t value\n\r\n".getBytes(); runTest(in); } @Test - public void testCanonicalPath() throws BadRequestException { - byte[] in = "GET\thttp://www.somehost.net/somepath\tHTTP/1.1\nHost: \t www.somehost.net\nOtherHeader:\tsome\n \t value\n\r\n".getBytes(); + public void testCanonicalPath() throws BadRequestException { + byte[] in = "GET http://www.somehost.net/somepath HTTP/1.1\nHost: \t www.somehost.net\nOtherHeader:\tsome\n \t value\n\r\n".getBytes(); final ParseState context = new ParseState(5); HttpServerExchange result = new HttpServerExchange(null); HttpRequestParser.instance(OptionMap.EMPTY).handle(ByteBuffer.wrap(in), context, result); @@ -183,7 +183,7 @@ public void testCanonicalPath() throws BadRequestException { @Test public void testNoHeaders() throws BadRequestException { - byte[] in = "GET\t/aa\tHTTP/1.1\n\n\n".getBytes(); + byte[] in = "GET /aa HTTP/1.1\n\n\n".getBytes(); final ParseState context = new ParseState(0); HttpServerExchange result = new HttpServerExchange(null); @@ -212,7 +212,7 @@ public void testQueryParams() throws BadRequestException { @Test public void testSameHttpStringReturned() throws BadRequestException { - byte[] in = "GET\thttp://www.somehost.net/somepath\tHTTP/1.1\nHost: \t www.somehost.net\nAccept-Charset:\tsome\n \t value\n\r\n".getBytes(); + byte[] in = "GET http://www.somehost.net/somepath HTTP/1.1\nHost: \t www.somehost.net\nAccept-Charset:\tsome\n \t value\n\r\n".getBytes(); final ParseState context1 = new ParseState(10); HttpServerExchange result1 = new HttpServerExchange(null); @@ -256,16 +256,14 @@ public void testEmptyQueryParams() throws BadRequestException { Assert.assertEquals("666", result.getQueryParameters().get("777").getFirst()); Assert.assertEquals("44", result.getQueryParameters().get(";?").getFirst()); } - @Test + + @Test(expected = BadRequestException.class) public void testNonEncodedAsciiCharacters() throws UnsupportedEncodingException, BadRequestException { byte[] in = "GET /bÃ¥r HTTP/1.1\r\n\r\n".getBytes("ISO-8859-1"); final ParseState context = new ParseState(10); HttpServerExchange result = new HttpServerExchange(null); HttpRequestParser.instance(OptionMap.EMPTY).handle(ByteBuffer.wrap(in), context, result); - Assert.assertSame(Methods.GET, result.getRequestMethod()); - Assert.assertEquals("/bår", result.getRequestPath()); - Assert.assertEquals("/bÃ¥r", result.getRequestURI()); //not decoded } private void runTest(final byte[] in) throws BadRequestException {
parser-generator/src/main/java/io/undertow/annotationprocessor/RequestParserGenerator.java+5 −0 modified@@ -32,6 +32,7 @@ public class RequestParserGenerator extends AbstractParserGenerator { public static final String PARSE_STATE_CLASS = "io.undertow.server.protocol.http.ParseState"; public static final String HTTP_EXCHANGE_CLASS = "io.undertow.server.HttpServerExchange"; public static final String HTTP_EXCHANGE_DESCRIPTOR = "Lio/undertow/server/HttpServerExchange;"; + private static final String CONNECTORS_CLASS = "io.undertow.server.Connectors"; //parsing states @@ -66,6 +67,8 @@ public boolean isHeader() { public void handleOtherToken(final CodeAttribute c) { c.aload(PARSE_STATE_VAR); c.swap(); + c.dup(); + c.invokestatic(CONNECTORS_CLASS, "verifyToken", "(" + HTTP_STRING_DESCRIPTOR + ")V"); c.putfield(parseStateClass, "nextHeader", HTTP_STRING_DESCRIPTOR); } @@ -150,6 +153,8 @@ public void handleStateMachineMatchedToken(final CodeAttribute c) { public void handleOtherToken(final CodeAttribute c) { c.aload(HTTP_RESULT); c.swap(); + c.dup(); + c.invokestatic(CONNECTORS_CLASS, "verifyToken", "(" + HTTP_STRING_DESCRIPTOR + ")V"); c.invokevirtual(resultClass, "setRequestMethod", "(" + HTTP_STRING_DESCRIPTOR + ")" + HTTP_EXCHANGE_DESCRIPTOR); c.pop(); }
servlet/src/test/java/io/undertow/servlet/test/path/PathFilter.java+1 −1 modified@@ -43,7 +43,7 @@ public void init(final FilterConfig filterConfig) throws ServletException { @Override public void doFilter(final ServletRequest request, final ServletResponse response, final FilterChain chain) throws IOException, ServletException { HttpServletResponse resp = (HttpServletResponse) response; - resp.addHeader("filter" + filterConfig.getFilterName(), filterConfig.getFilterName()); + resp.addHeader("filter" + filterConfig.getFilterName().replace("/", "-"), filterConfig.getFilterName()); chain.doFilter(request, response); }
Vulnerability mechanics
Generated by null/stub on May 9, 2026. Inputs: CWE entries + fix-commit diffs from this CVE's patches. Citations validated against bundle.
References
16- access.redhat.com/errata/RHSA-2017:3454mitrevendor-advisoryx_refsource_REDHAT
- access.redhat.com/errata/RHSA-2017:3455mitrevendor-advisoryx_refsource_REDHAT
- access.redhat.com/errata/RHSA-2017:3456mitrevendor-advisoryx_refsource_REDHAT
- access.redhat.com/errata/RHSA-2017:3458mitrevendor-advisoryx_refsource_REDHAT
- access.redhat.com/errata/RHSA-2018:0002mitrevendor-advisoryx_refsource_REDHAT
- access.redhat.com/errata/RHSA-2018:0003mitrevendor-advisoryx_refsource_REDHAT
- access.redhat.com/errata/RHSA-2018:0004mitrevendor-advisoryx_refsource_REDHAT
- access.redhat.com/errata/RHSA-2018:0005mitrevendor-advisoryx_refsource_REDHAT
- access.redhat.com/errata/RHSA-2018:1322mitrevendor-advisoryx_refsource_REDHAT
- github.com/advisories/GHSA-5gg7-5wv8-4gcjghsaADVISORY
- nvd.nist.gov/vuln/detail/CVE-2017-12165ghsaADVISORY
- bugzilla.redhat.com/show_bug.cgighsax_refsource_CONFIRMWEB
- github.com/undertow-io/undertow/commit/1e72647818c9fb31b693a953b1ae595a6c82eb7fghsaWEB
- github.com/undertow-io/undertow/commit/5b008b7ac312c6cdb76679ff58c43620bb79d44fghsaWEB
- github.com/undertow-io/undertow/commit/691440ee58259fba76711b60d56dde6679808bdcghsaWEB
- issues.redhat.com/browse/UNDERTOW-1251ghsaWEB
News mentions
0No linked articles in our index yet.