99 lines
4.7 KiB
Diff
99 lines
4.7 KiB
Diff
|
|
From: Norman Maurer <norman_maurer@apple.com>
|
||
|
|
Date: Fri, 20 Sep 2019 21:02:11 +0200
|
||
|
|
Subject: Correctly handle whitespaces in HTTP header names as defined by
|
||
|
|
RFC7230#section-3.2.4 (#9585)
|
||
|
|
Origin: https://github.com/netty/netty/commit/39cafcb05c99f2aa9fce7e6597664c9ed6a63a95
|
||
|
|
Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2019-16869
|
||
|
|
Bug-Debian: https://bugs.debian.org/941266
|
||
|
|
Bug: https://github.com/netty/netty/issues/9571
|
||
|
|
|
||
|
|
Motivation:
|
||
|
|
|
||
|
|
When parsing HTTP headers special care needs to be taken when a whitespace is detected in the header name.
|
||
|
|
|
||
|
|
Modifications:
|
||
|
|
|
||
|
|
- Ignore whitespace when decoding response (just like before)
|
||
|
|
- Throw exception when whitespace is detected during parsing
|
||
|
|
- Add unit tests
|
||
|
|
|
||
|
|
Result:
|
||
|
|
|
||
|
|
Fixes https://github.com/netty/netty/issues/9571
|
||
|
|
[Salvatore Bonaccorso: Backport to 4.1.7 for context changes in
|
||
|
|
HttpObjectDecoder.java]
|
||
|
|
---
|
||
|
|
.../handler/codec/http/HttpObjectDecoder.java | 16 +++++++++++++++-
|
||
|
|
.../codec/http/HttpRequestDecoderTest.java | 14 ++++++++++++++
|
||
|
|
.../codec/http/HttpResponseDecoderTest.java | 15 +++++++++++++++
|
||
|
|
3 files changed, 44 insertions(+), 1 deletion(-)
|
||
|
|
|
||
|
|
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java
|
||
|
|
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java
|
||
|
|
@@ -727,7 +727,21 @@ public abstract class HttpObjectDecoder
|
||
|
|
nameStart = findNonWhitespace(sb, 0);
|
||
|
|
for (nameEnd = nameStart; nameEnd < length; nameEnd ++) {
|
||
|
|
char ch = sb.charAt(nameEnd);
|
||
|
|
- if (ch == ':' || Character.isWhitespace(ch)) {
|
||
|
|
+ // https://tools.ietf.org/html/rfc7230#section-3.2.4
|
||
|
|
+ //
|
||
|
|
+ // No whitespace is allowed between the header field-name and colon. In
|
||
|
|
+ // the past, differences in the handling of such whitespace have led to
|
||
|
|
+ // security vulnerabilities in request routing and response handling. A
|
||
|
|
+ // server MUST reject any received request message that contains
|
||
|
|
+ // whitespace between a header field-name and colon with a response code
|
||
|
|
+ // of 400 (Bad Request). A proxy MUST remove any such whitespace from a
|
||
|
|
+ // response message before forwarding the message downstream.
|
||
|
|
+ if (ch == ':' ||
|
||
|
|
+ // In case of decoding a request we will just continue processing and header validation
|
||
|
|
+ // is done in the DefaultHttpHeaders implementation.
|
||
|
|
+ //
|
||
|
|
+ // In the case of decoding a response we will "skip" the whitespace.
|
||
|
|
+ (!isDecodingRequest() && Character.isWhitespace(ch))) {
|
||
|
|
break;
|
||
|
|
}
|
||
|
|
}
|
||
|
|
--- a/codec-http/src/test/java/io/netty/handler/codec/http/HttpRequestDecoderTest.java
|
||
|
|
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/HttpRequestDecoderTest.java
|
||
|
|
@@ -270,4 +270,18 @@ public class HttpRequestDecoderTest {
|
||
|
|
cnt.release();
|
||
|
|
assertFalse(channel.finishAndReleaseAll());
|
||
|
|
}
|
||
|
|
+
|
||
|
|
+ @Test
|
||
|
|
+ public void testWhitespace() {
|
||
|
|
+ EmbeddedChannel channel = new EmbeddedChannel(new HttpRequestDecoder());
|
||
|
|
+ String requestStr = "GET /some/path HTTP/1.1\r\n" +
|
||
|
|
+ "Transfer-Encoding : chunked\r\n" +
|
||
|
|
+ "Host: netty.io\n\r\n";
|
||
|
|
+
|
||
|
|
+ assertTrue(channel.writeInbound(Unpooled.copiedBuffer(requestStr, CharsetUtil.US_ASCII)));
|
||
|
|
+ HttpRequest request = channel.readInbound();
|
||
|
|
+ assertTrue(request.decoderResult().isFailure());
|
||
|
|
+ assertTrue(request.decoderResult().cause() instanceof IllegalArgumentException);
|
||
|
|
+ assertFalse(channel.finish());
|
||
|
|
+ }
|
||
|
|
}
|
||
|
|
--- a/codec-http/src/test/java/io/netty/handler/codec/http/HttpResponseDecoderTest.java
|
||
|
|
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/HttpResponseDecoderTest.java
|
||
|
|
@@ -661,4 +661,19 @@ public class HttpResponseDecoderTest {
|
||
|
|
assertThat(message.decoderResult().cause(), instanceOf(PrematureChannelClosureException.class));
|
||
|
|
assertNull(channel.readInbound());
|
||
|
|
}
|
||
|
|
+
|
||
|
|
+ @Test
|
||
|
|
+ public void testWhitespace() {
|
||
|
|
+ EmbeddedChannel channel = new EmbeddedChannel(new HttpResponseDecoder());
|
||
|
|
+ String requestStr = "HTTP/1.1 200 OK\r\n" +
|
||
|
|
+ "Transfer-Encoding : chunked\r\n" +
|
||
|
|
+ "Host: netty.io\n\r\n";
|
||
|
|
+
|
||
|
|
+ assertTrue(channel.writeInbound(Unpooled.copiedBuffer(requestStr, CharsetUtil.US_ASCII)));
|
||
|
|
+ HttpResponse response = channel.readInbound();
|
||
|
|
+ assertFalse(response.decoderResult().isFailure());
|
||
|
|
+ assertEquals(HttpHeaderValues.CHUNKED.toString(), response.headers().get(HttpHeaderNames.TRANSFER_ENCODING));
|
||
|
|
+ assertEquals("netty.io", response.headers().get(HttpHeaderNames.HOST));
|
||
|
|
+ assertFalse(channel.finish());
|
||
|
|
+ }
|
||
|
|
}
|