96 lines
4.7 KiB
Diff
96 lines
4.7 KiB
Diff
From 8494b046ec7e4f28dbd44bc699cc4c4c92251729 Mon Sep 17 00:00:00 2001
|
|
From: Norman Maurer <norman_maurer@apple.com>
|
|
Date: Fri, 13 Dec 2019 08:53:19 +0100
|
|
Subject: [PATCH] Verify we do not receive multiple content-length headers or a
|
|
content-length and transfer-encoding: chunked header when using HTTP/1.1
|
|
(#9865)
|
|
|
|
Motivation:
|
|
|
|
RFC7230 states that we should not accept multiple content-length headers and also should not accept a content-length header in combination with transfer-encoding: chunked
|
|
|
|
Modifications:
|
|
|
|
- Check for multiple content-length headers and if found mark message as invalid
|
|
- Check if we found a content-length header and also a transfer-encoding: chunked and if so mark the message as invalid
|
|
- Add unit test
|
|
|
|
Result:
|
|
|
|
Fixes https://github.com/netty/netty/issues/9861
|
|
---
|
|
.../handler/codec/http/HttpObjectDecoder.java | 50 +++++++++++++--
|
|
.../codec/http/HttpRequestDecoderTest.java | 64 ++++++++++++++++---
|
|
2 files changed, 99 insertions(+), 15 deletions(-)
|
|
|
|
--- a/src/main/java/org/jboss/netty/handler/codec/http/HttpMessageDecoder.java
|
|
+++ b/src/main/java/org/jboss/netty/handler/codec/http/HttpMessageDecoder.java
|
|
@@ -537,10 +537,30 @@
|
|
}
|
|
}
|
|
|
|
- State nextState;
|
|
+ List<String> values = message.headers().getAll(HttpHeaders.Names.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.getProtocolVersion() == HttpVersion.HTTP_1_1) {
|
|
+ throw new IllegalArgumentException("Multiple Content-Length headers found");
|
|
+ }
|
|
+ }
|
|
|
|
if (isContentAlwaysEmpty(message)) {
|
|
- nextState = State.SKIP_CONTROL_CHARS;
|
|
+ return State.SKIP_CONTROL_CHARS;
|
|
} else if (message.isChunked()) {
|
|
// HttpMessage.isChunked() returns true when either:
|
|
// 1) HttpMessage.setChunked(true) was called or
|
|
@@ -548,13 +568,29 @@
|
|
// Because this decoder did not call HttpMessage.setChunked(true)
|
|
// yet, HttpMessage.isChunked() should return true only when
|
|
// 'Transfer-Encoding' is 'chunked'.
|
|
- 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.getProtocolVersion() == HttpVersion.HTTP_1_1) {
|
|
+ throw new IllegalArgumentException(
|
|
+ "Both 'Content-Length: " + contentLength + "' and 'Transfer-Encoding: chunked' found");
|
|
+ }
|
|
+
|
|
+ return State.READ_CHUNK_SIZE;
|
|
} else if (HttpHeaders.getContentLength(message, -1) >= 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 HttpChunkTrailer readTrailingHeaders(ChannelBuffer buffer) throws TooLongFrameException {
|
|
|