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

313 lines
12 KiB
Diff
Raw Blame History

From 2d941652f20135719b9ae5f4a373328ce5379970 Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Thu, 9 Dec 2021 14:49:43 +0100
Subject: [PATCH] Merge pull request from GHSA-wx5j-54mm-rqqq
Motivation:
We should validate that only OWS is allowed before / after a header name and otherwise throw. At the moment we just "strip" everything except OWS.
Modifications:
- Adjust code to correctly validate
- Add unit tests
Result:
More strict and correct behaviour
---
.../codec/http/DefaultHttpHeaders.java | 8 ++
.../handler/codec/http/HttpObjectDecoder.java | 8 +-
.../codec/http/HttpRequestDecoderTest.java | 87 +++++++++++++++++--
.../codec/http/HttpResponseDecoderTest.java | 78 +++++++++++++++++
4 files changed, 171 insertions(+), 10 deletions(-)
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/DefaultHttpHeaders.java b/codec-http/src/main/java/io/netty/handler/codec/http/DefaultHttpHeaders.java
index d18f196..35dd88e 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/DefaultHttpHeaders.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/DefaultHttpHeaders.java
@@ -324,6 +324,10 @@ public class DefaultHttpHeaders extends HttpHeaders {
private static void validateHeaderNameElement(byte value) {
switch (value) {
+ case 0x1c:
+ case 0x1d:
+ case 0x1e:
+ case 0x1f:
case 0x00:
case '\t':
case '\n':
@@ -348,6 +352,10 @@ public class DefaultHttpHeaders extends HttpHeaders {
private static void validateHeaderNameElement(char value) {
switch (value) {
+ case 0x1c:
+ case 0x1d:
+ case 0x1e:
+ case 0x1f:
case 0x00:
case '\t':
case '\n':
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 b4de681..15e86a5 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
@@ -796,7 +796,7 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
int valueStart;
int valueEnd;
- nameStart = findNonWhitespace(sb, 0, false);
+ nameStart = findNonWhitespace(sb, 0);
for (nameEnd = nameStart; nameEnd < length; nameEnd ++) {
char ch = sb.charAt(nameEnd);
// https://tools.ietf.org/html/rfc7230#section-3.2.4
@@ -831,7 +831,7 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
}
name = sb.subStringUnsafe(nameStart, nameEnd);
- valueStart = findNonWhitespace(sb, colonEnd, true);
+ valueStart = findNonWhitespace(sb, colonEnd);
if (valueStart == length) {
value = EMPTY_VALUE;
} else {
@@ -870,12 +870,12 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
return c == ' ' || c == (char) 0x09 || c == (char) 0x0B || c == (char) 0x0C || c == (char) 0x0D;
}
- private static int findNonWhitespace(AppendableCharSequence sb, int offset, boolean validateOWS) {
+ private static int findNonWhitespace(AppendableCharSequence sb, int offset) {
for (int result = offset; result < sb.length(); ++result) {
char c = sb.charAtUnsafe(result);
if (!Character.isWhitespace(c)) {
return result;
- } else if (validateOWS && !isOWS(c)) {
+ } else if (!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 + "'");
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 902b379..fc7cfb4 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
@@ -15,6 +15,7 @@
*/
package io.netty.handler.codec.http;
+import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.netty.channel.embedded.EmbeddedChannel;
import io.netty.util.AsciiString;
@@ -294,6 +295,75 @@ public class HttpRequestDecoderTest {
assertFalse(channel.finishAndReleaseAll());
}
+ @Test
+ public void testHeaderNameStartsWithControlChar1c() {
+ testHeaderNameStartsWithControlChar(0x1c);
+ }
+
+ @Test
+ public void testHeaderNameStartsWithControlChar1d() {
+ testHeaderNameStartsWithControlChar(0x1d);
+ }
+
+ @Test
+ public void testHeaderNameStartsWithControlChar1e() {
+ testHeaderNameStartsWithControlChar(0x1e);
+ }
+
+ @Test
+ public void testHeaderNameStartsWithControlChar1f() {
+ testHeaderNameStartsWithControlChar(0x1f);
+ }
+
+ @Test
+ public void testHeaderNameStartsWithControlChar0c() {
+ testHeaderNameStartsWithControlChar(0x0c);
+ }
+
+ private void testHeaderNameStartsWithControlChar(int controlChar) {
+ ByteBuf requestBuffer = Unpooled.buffer();
+ requestBuffer.writeCharSequence("GET /some/path HTTP/1.1\r\n" +
+ "Host: netty.io\r\n", CharsetUtil.US_ASCII);
+ requestBuffer.writeByte(controlChar);
+ requestBuffer.writeCharSequence("Transfer-Encoding: chunked\r\n\r\n", CharsetUtil.US_ASCII);
+ testInvalidHeaders0(requestBuffer);
+ }
+
+ @Test
+ public void testHeaderNameEndsWithControlChar1c() {
+ testHeaderNameEndsWithControlChar(0x1c);
+ }
+
+ @Test
+ public void testHeaderNameEndsWithControlChar1d() {
+ testHeaderNameEndsWithControlChar(0x1d);
+ }
+
+ @Test
+ public void testHeaderNameEndsWithControlChar1e() {
+ testHeaderNameEndsWithControlChar(0x1e);
+ }
+
+ @Test
+ public void testHeaderNameEndsWithControlChar1f() {
+ testHeaderNameEndsWithControlChar(0x1f);
+ }
+
+ @Test
+ public void testHeaderNameEndsWithControlChar0c() {
+ testHeaderNameEndsWithControlChar(0x0c);
+ }
+
+ private void testHeaderNameEndsWithControlChar(int controlChar) {
+ ByteBuf requestBuffer = Unpooled.buffer();
+ requestBuffer.writeCharSequence("GET /some/path HTTP/1.1\r\n" +
+ "Host: netty.io\r\n", CharsetUtil.US_ASCII);
+ requestBuffer.writeCharSequence("Transfer-Encoding", CharsetUtil.US_ASCII);
+ requestBuffer.writeByte(controlChar);
+ requestBuffer.writeCharSequence(": chunked\r\n\r\n", CharsetUtil.US_ASCII);
+ testInvalidHeaders0(requestBuffer);
+ }
+
@Test
public void testWhitespace() {
String requestStr = "GET /some/path HTTP/1.1\r\n" +
@@ -303,9 +373,9 @@ public class HttpRequestDecoderTest {
}
@Test
- public void testWhitespaceBeforeTransferEncoding01() {
+ public void testWhitespaceInTransferEncoding01() {
String requestStr = "GET /some/path HTTP/1.1\r\n" +
- " Transfer-Encoding : chunked\r\n" +
+ "Transfer-Encoding : chunked\r\n" +
"Content-Length: 1\r\n" +
"Host: netty.io\r\n\r\n" +
"a";
@@ -313,9 +383,9 @@ public class HttpRequestDecoderTest {
}
@Test
- public void testWhitespaceBeforeTransferEncoding02() {
+ public void testWhitespaceInTransferEncoding02() {
String requestStr = "POST / HTTP/1.1" +
- " Transfer-Encoding : chunked\r\n" +
+ "Transfer-Encoding : chunked\r\n" +
"Host: target.com" +
"Content-Length: 65\r\n\r\n" +
"0\r\n\r\n" +
@@ -412,15 +482,20 @@ public class HttpRequestDecoderTest {
assertTrue(request.headers().contains("Transfer-Encoding", "chunked", false));
assertFalse(request.headers().contains("Content-Length"));
LastHttpContent c = channel.readInbound();
+ c.release();
assertFalse(channel.finish());
}
private static void testInvalidHeaders0(String requestStr) {
+ testInvalidHeaders0(Unpooled.copiedBuffer(requestStr, CharsetUtil.US_ASCII));
+ }
+
+ private static void testInvalidHeaders0(ByteBuf requestBuffer) {
EmbeddedChannel channel = new EmbeddedChannel(new HttpRequestDecoder());
- assertTrue(channel.writeInbound(Unpooled.copiedBuffer(requestStr, CharsetUtil.US_ASCII)));
+ assertTrue(channel.writeInbound(requestBuffer));
HttpRequest request = channel.readInbound();
+ assertThat(request.decoderResult().cause(), instanceOf(IllegalArgumentException.class));
assertTrue(request.decoderResult().isFailure());
- assertTrue(request.decoderResult().cause() instanceof IllegalArgumentException);
assertFalse(channel.finish());
}
}
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 66cefc9..df44293 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
@@ -676,4 +676,82 @@ public class HttpResponseDecoderTest {
assertEquals("netty.io", response.headers().get(HttpHeaderNames.HOST));
assertFalse(channel.finish());
}
+
+ @Test
+ public void testHeaderNameStartsWithControlChar1c() {
+ testHeaderNameStartsWithControlChar(0x1c);
+ }
+
+ @Test
+ public void testHeaderNameStartsWithControlChar1d() {
+ testHeaderNameStartsWithControlChar(0x1d);
+ }
+
+ @Test
+ public void testHeaderNameStartsWithControlChar1e() {
+ testHeaderNameStartsWithControlChar(0x1e);
+ }
+
+ @Test
+ public void testHeaderNameStartsWithControlChar1f() {
+ testHeaderNameStartsWithControlChar(0x1f);
+ }
+
+ @Test
+ public void testHeaderNameStartsWithControlChar0c() {
+ testHeaderNameStartsWithControlChar(0x0c);
+ }
+
+ private void testHeaderNameStartsWithControlChar(int controlChar) {
+ ByteBuf responseBuffer = Unpooled.buffer();
+ responseBuffer.writeCharSequence("HTTP/1.1 200 OK\r\n" +
+ "Host: netty.io\r\n", CharsetUtil.US_ASCII);
+ responseBuffer.writeByte(controlChar);
+ responseBuffer.writeCharSequence("Transfer-Encoding: chunked\r\n\r\n", CharsetUtil.US_ASCII);
+ testInvalidHeaders0(responseBuffer);
+ }
+
+ @Test
+ public void testHeaderNameEndsWithControlChar1c() {
+ testHeaderNameEndsWithControlChar(0x1c);
+ }
+
+ @Test
+ public void testHeaderNameEndsWithControlChar1d() {
+ testHeaderNameEndsWithControlChar(0x1d);
+ }
+
+ @Test
+ public void testHeaderNameEndsWithControlChar1e() {
+ testHeaderNameEndsWithControlChar(0x1e);
+ }
+
+ @Test
+ public void testHeaderNameEndsWithControlChar1f() {
+ testHeaderNameEndsWithControlChar(0x1f);
+ }
+
+ @Test
+ public void testHeaderNameEndsWithControlChar0c() {
+ testHeaderNameEndsWithControlChar(0x0c);
+ }
+
+ private void testHeaderNameEndsWithControlChar(int controlChar) {
+ ByteBuf responseBuffer = Unpooled.buffer();
+ responseBuffer.writeCharSequence("HTTP/1.1 200 OK\r\n" +
+ "Host: netty.io\r\n", CharsetUtil.US_ASCII);
+ responseBuffer.writeCharSequence("Transfer-Encoding", CharsetUtil.US_ASCII);
+ responseBuffer.writeByte(controlChar);
+ responseBuffer.writeCharSequence(": chunked\r\n\r\n", CharsetUtil.US_ASCII);
+ testInvalidHeaders0(responseBuffer);
+ }
+
+ private static void testInvalidHeaders0(ByteBuf responseBuffer) {
+ EmbeddedChannel channel = new EmbeddedChannel(new HttpResponseDecoder());
+ assertTrue(channel.writeInbound(responseBuffer));
+ HttpResponse response = channel.readInbound();
+ assertThat(response.decoderResult().cause(), instanceOf(IllegalArgumentException.class));
+ assertTrue(response.decoderResult().isFailure());
+ assertFalse(channel.finish());
+ }
}
--
2.27.0
Reference in New Issue View Git Blame Copy Permalink