packages/netty
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity
netty/CVE-2021-43797-pre.patch

232 lines
9.9 KiB
Diff
Raw Permalink Blame History

From 74187ebf123de466cb31270213b2464267a1cfd4 Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Wed, 26 Feb 2020 09:49:39 +0100
Subject: [PATCH 1/1] More strict parsing of initial line / http headers
(#10058)
Motivation:
Our parsing of the initial line / http headers did treat some characters as separators which should better trigger an exception during parsing.
Modifications:
- Tighten up parsing of the inital line by follow recommentation of RFC7230
- Restrict separators to OWS for http headers
- Add unit test
Result:
Stricter parsing of HTTP1
---
.../handler/codec/http/HttpObjectDecoder.java | 63 ++++++++++++++-----
.../codec/http/HttpRequestDecoderTest.java | 25 +++++++-
.../codec/http/HttpResponseDecoderTest.java | 2 +-
.../util/internal/AppendableCharSequence.java | 7 +++
4 files changed, 80 insertions(+), 17 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 82b0c36..b4de681 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
@@ -773,13 +773,13 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
int cStart;
int cEnd;
- aStart = findNonWhitespace(sb, 0);
- aEnd = findWhitespace(sb, aStart);
+ aStart = findNonSPLenient(sb, 0);
+ aEnd = findSPLenient(sb, aStart);
- bStart = findNonWhitespace(sb, aEnd);
- bEnd = findWhitespace(sb, bStart);
+ bStart = findNonSPLenient(sb, aEnd);
+ bEnd = findSPLenient(sb, bStart);
- cStart = findNonWhitespace(sb, bEnd);
+ cStart = findNonSPLenient(sb, bEnd);
cEnd = findEndOfString(sb);
return new String[] {
@@ -796,7 +796,7 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
int valueStart;
int valueEnd;
- nameStart = findNonWhitespace(sb, 0);
+ nameStart = findNonWhitespace(sb, 0, false);
for (nameEnd = nameStart; nameEnd < length; nameEnd ++) {
char ch = sb.charAt(nameEnd);
// https://tools.ietf.org/html/rfc7230#section-3.2.4
@@ -813,7 +813,7 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
// is done in the DefaultHttpHeaders implementation.
//
// In the case of decoding a response we will "skip" the whitespace.
- (!isDecodingRequest() && Character.isWhitespace(ch))) {
+ (!isDecodingRequest() && isOWS(ch))) {
break;
}
}
@@ -831,7 +831,7 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
}
name = sb.subStringUnsafe(nameStart, nameEnd);
- valueStart = findNonWhitespace(sb, colonEnd);
+ valueStart = findNonWhitespace(sb, colonEnd, true);
if (valueStart == length) {
value = EMPTY_VALUE;
} else {
@@ -840,19 +840,45 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
}
}
- private static int findNonWhitespace(AppendableCharSequence sb, int offset) {
+ private static int findNonSPLenient(AppendableCharSequence sb, int offset) {
for (int result = offset; result < sb.length(); ++result) {
- if (!Character.isWhitespace(sb.charAtUnsafe(result))) {
+ char c = sb.charAtUnsafe(result);
+ // See https://tools.ietf.org/html/rfc7230#section-3.5
+ if (isSPLenient(c)) {
+ continue;
+ }
+ if (Character.isWhitespace(c)) {
+ // Any other whitespace delimiter is invalid
+ throw new IllegalArgumentException("Invalid separator");
+ }
+ return result;
+ }
+ return sb.length();
+ }
+
+ private static int findSPLenient(AppendableCharSequence sb, int offset) {
+ for (int result = offset; result < sb.length(); ++result) {
+ if (isSPLenient(sb.charAtUnsafe(result))) {
return result;
}
}
return sb.length();
}
- private static int findWhitespace(AppendableCharSequence sb, int offset) {
+ private static boolean isSPLenient(char c) {
+ // See https://tools.ietf.org/html/rfc7230#section-3.5
+ return c == ' ' || c == (char) 0x09 || c == (char) 0x0B || c == (char) 0x0C || c == (char) 0x0D;
+ }
+
+ private static int findNonWhitespace(AppendableCharSequence sb, int offset, boolean validateOWS) {
for (int result = offset; result < sb.length(); ++result) {
- if (Character.isWhitespace(sb.charAtUnsafe(result))) {
+ char c = sb.charAtUnsafe(result);
+ if (!Character.isWhitespace(c)) {
return result;
+ } else if (validateOWS && !isOWS(c)) {
+ // Only OWS is supported for whitespace
+ throw new IllegalArgumentException("Invalid separator, only a single space or horizontal tab allowed," +
+ " but received a '" + c + "'");
}
}
return sb.length();
@@ -867,6 +893,10 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
return 0;
}
+ private static boolean isOWS(char ch) {
+ return ch == ' ' || ch == (char) 0x09;
+ }
+
private static class HeaderParser implements ByteProcessor {
private final AppendableCharSequence seq;
private final int maxLength;
@@ -896,10 +926,13 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
@Override
public boolean process(byte value) throws Exception {
char nextByte = (char) (value & 0xFF);
- if (nextByte == HttpConstants.CR) {
- return true;
- }
if (nextByte == HttpConstants.LF) {
+ int len = seq.length();
+ // Drop CR if we had a CRLF pair
+ if (len >= 1 && seq.charAtUnsafe(len - 1) == HttpConstants.CR) {
+ -- size;
+ seq.setLength(len - 1);
+ }
return false;
}
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 000bd0c..902b379 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
@@ -373,6 +373,30 @@ public class HttpRequestDecoderTest {
testInvalidHeaders0(requestStr);
}
+ @Test
+ public void testContentLengthAndTransferEncodingHeadersWithVerticalTab() {
+ testContentLengthAndTransferEncodingHeadersWithInvalidSeparator((char) 0x0b, false);
+ testContentLengthAndTransferEncodingHeadersWithInvalidSeparator((char) 0x0b, true);
+ }
+
+ @Test
+ public void testContentLengthAndTransferEncodingHeadersWithCR() {
+ testContentLengthAndTransferEncodingHeadersWithInvalidSeparator((char) 0x0d, false);
+ testContentLengthAndTransferEncodingHeadersWithInvalidSeparator((char) 0x0d, true);
+ }
+
+ private static void testContentLengthAndTransferEncodingHeadersWithInvalidSeparator(
+ char separator, boolean extraLine) {
+ String requestStr = "POST / HTTP/1.1\r\n" +
+ "Host: example.com\r\n" +
+ "Connection: close\r\n" +
+ "Content-Length: 9\r\n" +
+ "Transfer-Encoding:" + separator + "chunked\r\n\r\n" +
+ (extraLine ? "0\r\n\r\n" : "") +
+ "something\r\n\r\n";
+ testInvalidHeaders0(requestStr);
+ }
+
@Test
public void testContentLengthHeaderAndChunked() {
String requestStr = "POST / HTTP/1.1\r\n" +
@@ -381,7 +405,6 @@ public class HttpRequestDecoderTest {
"Content-Length: 5\r\n" +
"Transfer-Encoding: chunked\r\n\r\n" +
"0\r\n\r\n";
-
EmbeddedChannel channel = new EmbeddedChannel(new HttpRequestDecoder());
assertTrue(channel.writeInbound(Unpooled.copiedBuffer(requestStr, CharsetUtil.US_ASCII)));
HttpRequest request = channel.readInbound();
diff --git 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
index d67b3ad..66cefc9 100644
--- 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
@@ -50,7 +50,7 @@ public class HttpResponseDecoderTest {
final int maxHeaderSize = 8192;
final EmbeddedChannel ch = new EmbeddedChannel(new HttpResponseDecoder(4096, maxHeaderSize, 8192));
- final char[] bytes = new char[maxHeaderSize / 2 - 2];
+ final char[] bytes = new char[maxHeaderSize / 2 - 4];
Arrays.fill(bytes, 'a');
ch.writeInbound(Unpooled.copiedBuffer("HTTP/1.1 200 OK\r\n", CharsetUtil.US_ASCII));
diff --git a/common/src/main/java/io/netty/util/internal/AppendableCharSequence.java b/common/src/main/java/io/netty/util/internal/AppendableCharSequence.java
index 408c32f..bf1eee5 100644
--- a/common/src/main/java/io/netty/util/internal/AppendableCharSequence.java
+++ b/common/src/main/java/io/netty/util/internal/AppendableCharSequence.java
@@ -37,6 +37,13 @@ public final class AppendableCharSequence implements CharSequence, Appendable {
pos = chars.length;
}
+ public void setLength(int length) {
+ if (length < 0 || length > pos) {
+ throw new IllegalArgumentException("length: " + length + " (length: >= 0, <= " + pos + ')');
+ }
+ this.pos = length;
+ }
+
@Override
public int length() {
return pos;
--
2.27.0
Reference in New Issue View Git Blame Copy Permalink