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
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
From: Markus Koschany <apo@debian.org>
Date: Sat, 20 Mar 2021 13:48:07 +0200
Subject: CVE-2019-20445_1
This is also the fix for CVE-2020-7238.
Bug-Debian: https://bugs.debian.org/950967
Origin: https://github.com/netty/netty/commit/8494b046ec7e4f28dbd44bc699cc4c4c92251729
---
.../handler/codec/http/HttpObjectDecoder.java | 50 +++++++++++++++--
.../handler/codec/http/HttpRequestDecoderTest.java | 64 +++++++++++++++++++---
2 files changed, 99 insertions(+), 15 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 d3f5b79..0a9ea14 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
@@ -609,23 +609,61 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
if (name != null) {
headers.add(name, value);
}
+
// reset name and value fields
name = null;
value = null;
- State nextState;
+ List<String> values = headers.getAll(HttpHeaderNames.CONTENT_LENGTH);
+ int contentLengthValuesCount = values.size();
+
+ if (contentLengthValuesCount > 0) {
+ // Guard against multiple Content-Length headers as stated in
+ // https://tools.ietf.org/html/rfc7230#section-3.3.2:
+ //
+ // If a message is received that has multiple Content-Length header
+ // fields with field-values consisting of the same decimal value, or a
+ // single Content-Length header field with a field value containing a
+ // list of identical decimal values (e.g., "Content-Length: 42, 42"),
+ // indicating that duplicate Content-Length header fields have been
+ // generated or combined by an upstream message processor, then the
+ // recipient MUST either reject the message as invalid or replace the
+ // duplicated field-values with a single valid Content-Length field
+ // containing that decimal value prior to determining the message body
+ // length or forwarding the message.
+ if (contentLengthValuesCount > 1 && message.protocolVersion() == HttpVersion.HTTP_1_1) {
+ throw new IllegalArgumentException("Multiple Content-Length headers found");
+ }
+ contentLength = Long.parseLong(values.get(0));
+ }
if (isContentAlwaysEmpty(message)) {
HttpUtil.setTransferEncodingChunked(message, false);
- nextState = State.SKIP_CONTROL_CHARS;
+ return State.SKIP_CONTROL_CHARS;
} else if (HttpUtil.isTransferEncodingChunked(message)) {
- nextState = State.READ_CHUNK_SIZE;
+ // 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");
+ }
+
+ return State.READ_CHUNK_SIZE;
} else if (contentLength() >= 0) {
- nextState = State.READ_FIXED_LENGTH_CONTENT;
+ return State.READ_FIXED_LENGTH_CONTENT;
} else {
- nextState = State.READ_VARIABLE_LENGTH_CONTENT;
+ return State.READ_VARIABLE_LENGTH_CONTENT;
}
- return nextState;
}
private long contentLength() {
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 414a033..717b580 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
@@ -323,29 +323,75 @@ public class HttpRequestDecoderTest {
@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());
+ testInvalidHeaders0(requestStr);
}
@Test
public void testHeaderWithNoValueAndMissingColon() {
- EmbeddedChannel channel = new EmbeddedChannel(new HttpRequestDecoder());
String requestStr = "GET /some/path HTTP/1.1\r\n" +
"Content-Length: 0\r\n" +
"Host:\r\n" +
"netty.io\r\n\r\n";
+ testInvalidHeaders0(requestStr);
+ }
+
+ @Test
+ public void testMultipleContentLengthHeaders() {
+ String requestStr = "GET /some/path HTTP/1.1\r\n" +
+ "Content-Length: 1\r\n" +
+ "Content-Length: 0\r\n\r\n" +
+ "b";
+ testInvalidHeaders0(requestStr);
+ }
+
+ @Test
+ public void testMultipleContentLengthHeaders2() {
+ String requestStr = "GET /some/path HTTP/1.1\r\n" +
+ "Content-Length: 1\r\n" +
+ "Connection: close\r\n" +
+ "Content-Length: 0\r\n\r\n" +
+ "b";
+ testInvalidHeaders0(requestStr);
+ }
+
+ @Test
+ public void testContentLengthHeaderWithCommaValue() {
+ String requestStr = "GET /some/path HTTP/1.1\r\n" +
+ "Content-Length: 1,1\r\n\r\n" +
+ "b";
+ testInvalidHeaders0(requestStr);
+ }
+ @Test
+ public void testMultipleContentLengthHeadersWithFolding() {
+ String requestStr = "POST / HTTP/1.1\r\n" +
+ "Host: example.com\r\n" +
+ "Connection: close\r\n" +
+ "Content-Length: 5\r\n" +
+ "Content-Length:\r\n" +
+ "\t6\r\n\r\n" +
+ "123456";
+ testInvalidHeaders0(requestStr);
+ }
+
+ @Test
+ public void testContentLengthHeaderAndChunked() {
+ String requestStr = "POST / HTTP/1.1\r\n" +
+ "Host: example.com\r\n" +
+ "Connection: close\r\n" +
+ "Content-Length: 5\r\n" +
+ "Transfer-Encoding: chunked\r\n\r\n" +
+ "0\r\n\r\n";
+ testInvalidHeaders0(requestStr);
+ }
+
+ private static void testInvalidHeaders0(String requestStr) {
+ EmbeddedChannel channel = new EmbeddedChannel(new HttpRequestDecoder());
assertTrue(channel.writeInbound(Unpooled.copiedBuffer(requestStr, CharsetUtil.US_ASCII)));
HttpRequest request = channel.readInbound();
- System.err.println(request.headers().names().toString());
assertTrue(request.decoderResult().isFailure());
assertTrue(request.decoderResult().cause() instanceof IllegalArgumentException);
assertFalse(channel.finish());