Newer
Older
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
From: Markus Koschany <apo@debian.org>
Date: Sat, 20 Mar 2021 13:48:44 +0200
Subject: CVE-2019-20445_2
Origin: https://github.com/netty/netty/commit/629034624626b722128e0fcc6b3ec9d406cb3706
---
.../handler/codec/http/HttpObjectDecoder.java | 42 ++++++++++++++--------
.../handler/codec/http/HttpRequestDecoderTest.java | 10 +++++-
2 files changed, 36 insertions(+), 16 deletions(-)
diff --git 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
index 0a9ea14..f81880c 100644
--- 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
@@ -641,23 +641,9 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
HttpUtil.setTransferEncodingChunked(message, false);
return State.SKIP_CONTROL_CHARS;
} else if (HttpUtil.isTransferEncodingChunked(message)) {
- // See https://tools.ietf.org/html/rfc7230#section-3.3.3
- //
- // If a message is received with both a Transfer-Encoding and a
- // Content-Length header field, the Transfer-Encoding overrides the
- // Content-Length. Such a message might indicate an attempt to
- // perform request smuggling (Section 9.5) or response splitting
- // (Section 9.4) and ought to be handled as an error. A sender MUST
- // remove the received Content-Length field prior to forwarding such
- // a message downstream.
- //
- // This is also what http_parser does:
- // https://github.com/nodejs/http-parser/blob/v2.9.2/http_parser.c#L1769
if (contentLengthValuesCount > 0 && message.protocolVersion() == HttpVersion.HTTP_1_1) {
- throw new IllegalArgumentException(
- "Both 'Content-Length: " + contentLength + "' and 'Transfer-Encoding: chunked' found");
+ handleTransferEncodingChunkedWithContentLength(message);
}
-
return State.READ_CHUNK_SIZE;
} else if (contentLength() >= 0) {
return State.READ_FIXED_LENGTH_CONTENT;
@@ -666,6 +652,32 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
}
}
+ /**
+ * Invoked when a message with both a "Transfer-Encoding: chunked" and a "Content-Length" header field is detected.
+ * The default behavior is to <i>remove</i> the Content-Length field, but this method could be overridden
+ * to change the behavior (to, e.g., throw an exception and produce an invalid message).
+ * <p>
+ * See: https://tools.ietf.org/html/rfc7230#section-3.3.3
+ * <pre>
+ * If a message is received with both a Transfer-Encoding and a
+ * Content-Length header field, the Transfer-Encoding overrides the
+ * Content-Length. Such a message might indicate an attempt to
+ * perform request smuggling (Section 9.5) or response splitting
+ * (Section 9.4) and ought to be handled as an error. A sender MUST
+ * remove the received Content-Length field prior to forwarding such
+ * a message downstream.
+ * </pre>
+ * Also see:
+ * https://github.com/apache/tomcat/blob/b693d7c1981fa7f51e58bc8c8e72e3fe80b7b773/
+ * java/org/apache/coyote/http11/Http11Processor.java#L747-L755
+ * https://github.com/nginx/nginx/blob/0ad4393e30c119d250415cb769e3d8bc8dce5186/
+ * src/http/ngx_http_request.c#L1946-L1953
+ */
+ protected void handleTransferEncodingChunkedWithContentLength(HttpMessage message) {
+ message.headers().remove(HttpHeaderNames.CONTENT_LENGTH);
+ contentLength = Long.MIN_VALUE;
+ }
+
private long contentLength() {
if (contentLength == Long.MIN_VALUE) {
contentLength = HttpUtil.getContentLength(message, -1L);
diff --git 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
index 717b580..5aa6fec 100644
--- 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
@@ -385,7 +385,15 @@ public class HttpRequestDecoderTest {
"Content-Length: 5\r\n" +
"Transfer-Encoding: chunked\r\n\r\n" +
"0\r\n\r\n";
- testInvalidHeaders0(requestStr);
+
+ EmbeddedChannel channel = new EmbeddedChannel(new HttpRequestDecoder());
+ assertTrue(channel.writeInbound(Unpooled.copiedBuffer(requestStr, CharsetUtil.US_ASCII)));
+ HttpRequest request = channel.readInbound();
+ assertFalse(request.decoderResult().isFailure());
+ assertTrue(request.headers().contains("Transfer-Encoding", "chunked", false));
+ assertFalse(request.headers().contains("Content-Length"));
+ LastHttpContent c = channel.readInbound();
+ assertFalse(channel.finish());
}
private static void testInvalidHeaders0(String requestStr) {