On Thu, Jan 02, 2020 at 08:36:03PM -0800, tony mancill wrote: > On Thu, Jan 02, 2020 at 11:38:08PM +0100, Salvatore Bonaccorso wrote: > > On Fri, Sep 27, 2019 at 01:12:04PM +0200, Salvatore Bonaccorso wrote: > > > Source: netty > > > Version: 1:4.1.33-1 > > > Severity: important > > > Tags: security upstream > > > Forwarded: https://github.com/netty/netty/issues/9571 > > > > > > Hi, > > > > > > The following vulnerability was published for netty. > > > > > > CVE-2019-16869[0]: > > > | Netty before 4.1.42.Final mishandles whitespace before the colon in > > > | HTTP headers (such as a "Transfer-Encoding : chunked" line), which > > > | leads to HTTP request smuggling. > > > > Attached is the proposed debdiff. I included the tests as well > > (altough those are not run). > > Hi Salvatore, > > The debdiff looks good to me; thank you for adapting the patch for the > current version in 4.1.33. No need for an NMU. I will apply your patch > and perform a team upload to unstable with only this change to make it > easier for backports/security uploads.
Many thanks! The debdiffs for stretch- and buster-security are attached (but DSA not yet released). Regards, Salvatore
diff -Nru netty-4.1.7/debian/changelog netty-4.1.7/debian/changelog
--- netty-4.1.7/debian/changelog 2017-01-23 09:32:14.000000000 +0100
+++ netty-4.1.7/debian/changelog 2020-01-02 23:46:59.000000000 +0100
@@ -1,3 +1,11 @@
+netty (1:4.1.7-2+deb9u1) stretch-security; urgency=high
+
+ * Non-maintainer upload by the Security Team.
+ * Correctly handle whitespaces in HTTP header names as defined by
+ RFC7230#section-3.2.4 (CVE-2019-16869) (Closes: #941266)
+
+ -- Salvatore Bonaccorso <car...@debian.org> Thu, 02 Jan 2020 23:46:59 +0100
+
netty (1:4.1.7-2) unstable; urgency=medium
* Team upload.
diff -Nru
netty-4.1.7/debian/patches/14-Correctly-handle-whitespaces-in-HTTP-header-names-as.patch
netty-4.1.7/debian/patches/14-Correctly-handle-whitespaces-in-HTTP-header-names-as.patch
---
netty-4.1.7/debian/patches/14-Correctly-handle-whitespaces-in-HTTP-header-names-as.patch
1970-01-01 01:00:00.000000000 +0100
+++
netty-4.1.7/debian/patches/14-Correctly-handle-whitespaces-in-HTTP-header-names-as.patch
2020-01-02 23:46:59.000000000 +0100
@@ -0,0 +1,98 @@
+From: Norman Maurer <norman_mau...@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());
++ }
+ }
diff -Nru netty-4.1.7/debian/patches/series netty-4.1.7/debian/patches/series
--- netty-4.1.7/debian/patches/series 2017-01-16 09:05:15.000000000 +0100
+++ netty-4.1.7/debian/patches/series 2020-01-02 23:46:59.000000000 +0100
@@ -9,3 +9,4 @@
09-ignore-lz4.patch
10-ignore-lzma.patch
11-ignore-protobuf-nano.patch
+14-Correctly-handle-whitespaces-in-HTTP-header-names-as.patch
diff -Nru netty-4.1.33/debian/changelog netty-4.1.33/debian/changelog --- netty-4.1.33/debian/changelog 2019-01-22 14:06:25.000000000 +0100 +++ netty-4.1.33/debian/changelog 2020-01-02 23:19:52.000000000 +0100 @@ -1,3 +1,11 @@ +netty (1:4.1.33-1+deb10u1) buster-security; urgency=high + + * Non-maintainer upload by the Security Team. + * Correctly handle whitespaces in HTTP header names as defined by + RFC7230#section-3.2.4 (CVE-2019-16869) (Closes: #941266) + + -- Salvatore Bonaccorso <car...@debian.org> Thu, 02 Jan 2020 23:19:52 +0100 + netty (1:4.1.33-1) unstable; urgency=medium * Team upload. diff -Nru netty-4.1.33/debian/patches/14-Correctly-handle-whitespaces-in-HTTP-header-names-as.patch netty-4.1.33/debian/patches/14-Correctly-handle-whitespaces-in-HTTP-header-names-as.patch --- netty-4.1.33/debian/patches/14-Correctly-handle-whitespaces-in-HTTP-header-names-as.patch 1970-01-01 01:00:00.000000000 +0100 +++ netty-4.1.33/debian/patches/14-Correctly-handle-whitespaces-in-HTTP-header-names-as.patch 2020-01-02 23:19:52.000000000 +0100 @@ -0,0 +1,98 @@ +From: Norman Maurer <norman_mau...@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.33 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 +@@ -736,7 +736,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 +@@ -320,4 +320,18 @@ public class HttpRequestDecoderTest { + assertTrue(request.decoderResult().cause() instanceof TooLongFrameException); + assertFalse(channel.finish()); + } ++ ++ @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 +@@ -683,4 +683,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()); ++ } + } diff -Nru netty-4.1.33/debian/patches/series netty-4.1.33/debian/patches/series --- netty-4.1.33/debian/patches/series 2019-01-22 11:10:37.000000000 +0100 +++ netty-4.1.33/debian/patches/series 2020-01-02 23:19:52.000000000 +0100 @@ -9,3 +9,4 @@ 10-ignore-lzma.patch 11-ignore-protobuf-nano.patch 13-ignore-conscrypt.patch +14-Correctly-handle-whitespaces-in-HTTP-header-names-as.patch
signature.asc
Description: PGP signature
