CVE-2021-43797: Inconsistent Interpretation of HTTP Requests ('HTTP Request/Response Smuggling') (CWE-444)
value: Netty is an asynchronous event-driven network application framework for rapid development of maintainable high performance protocol servers & clients. Netty prior to version 4.1.71.Final skips control chars when they are present at the beginning / end of the header name. It should instead fail fast as these are not allowed by the spec and could lead to HTTP request smuggling. Failing to do the validation might cause netty to "sanitize" header names before it forward these to another remote system when used as proxy. This remote system can't see the invalid usage anymore, and therefore does not do the validation itself. Users should upgrade to version 4.1.71.Final.
CWE description: The product acts as an intermediary HTTP agent
         (such as a proxy or firewall) in the data flow between two
         entities such as a client and server, but it does not
         interpret malformed HTTP requests or responses in ways that
         are consistent with how the messages will be processed by
         those entities that are at the ultimate destination.
CVSS v3 base score: 6.5
Published: 2021-12-09T19:15Z
Repository: https://github.com/netty/netty
Language: Java
Files changed: codec-http/src/main/java/io/netty/handler/codec/http/DefaultHttpHeaders.java, codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java, codec-http/src/test/java/io/netty/handler/codec/http/HttpRequestDecoderTest.java, codec-http/src/test/java/io/netty/handler/codec/http/HttpResponseDecoderTest.java
Commit date: 2021-12-09 14:49:43 +0100
Commit message: 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
Diff: 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 ddd68c74d9..a728e678ba 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
@@ -365,10 +365,14 @@ public class DefaultHttpHeaders extends HttpHeaders {
         return new DefaultHttpHeaders(headers.copy());
     }
 
     private static void validateHeaderNameElement(byte value) {
         switch (value) {
+        case 0x1c:
+        case 0x1d:
+        case 0x1e:
+        case 0x1f:
         case 0x00:
         case '\t':
         case '\n':
         case 0x0b:
         case '\f':
@@ -389,10 +393,14 @@ 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':
         case 0x0b:
         case '\f':
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 cd2b496966..974dafe190 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
@@ -822,11 +822,11 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
         int nameEnd;
         int colonEnd;
         int valueStart;
         int valueEnd;
 
-        nameStart = findNonWhitespace(sb, 0, false);
+        nameStart = findNonWhitespace(sb, 0);
         for (nameEnd = nameStart; nameEnd < length; nameEnd ++) {
             char ch = sb.charAtUnsafe(nameEnd);
             // https://tools.ietf.org/html/rfc7230#section-3.2.4
             //
             // No whitespace is allowed between the header field-name and colon. In
@@ -857,11 +857,11 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
                 break;
             }
         }
 
         name = sb.subStringUnsafe(nameStart, nameEnd);
-        valueStart = findNonWhitespace(sb, colonEnd, true);
+        valueStart = findNonWhitespace(sb, colonEnd);
         if (valueStart == length) {
             value = EMPTY_VALUE;
         } else {
             valueEnd = findEndOfString(sb);
             value = sb.subStringUnsafe(valueStart, valueEnd);
@@ -896,16 +896,16 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
     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) {
+    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 + "' (0x" + Integer.toHexString(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 9edd6929c3..cac2f867ec 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
@@ -13,10 +13,11 @@
  * License for the specific language governing permissions and limitations
  * under the License.
  */
 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.handler.codec.TooLongFrameException;
 import io.netty.util.AsciiString;
 import io.netty.util.CharsetUtil;
@@ -355,32 +356,101 @@ public class HttpRequestDecoderTest {
         assertTrue(request.decoderResult().isFailure());
         assertTrue(request.decoderResult().cause() instanceof TooLongFrameException);
         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 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" +
                 "Transfer-Encoding : chunked\r\n" +
                 "Host: netty.io\r\n\r\n";
         testInvalidHeaders0(requestStr);
     }
 
     @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";
         testInvalidHeaders0(requestStr);
     }
 
     @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" +
                 "GET /maliciousRequest HTTP/1.1\r\n" +
                 "Host: evilServer.com\r\n" +
@@ -473,10 +543,11 @@ public class HttpRequestDecoderTest {
         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();
+        c.release();
         assertFalse(channel.finish());
     }
 
     @Test
     public void testHttpMessageDecoderResult() {
@@ -497,13 +568,17 @@ public class HttpRequestDecoderTest {
         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 de93638925..7d4fe92a5d 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
@@ -797,6 +797,84 @@ public class HttpResponseDecoderTest {
         assertThat(decoderResult.totalSize(), is(50));
         HttpContent c = channel.readInbound();
         c.release();
         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());
+    }
 }

Vulnerable code:         nameStart = findNonWhitespace(sb, 0, false);
        valueStart = findNonWhitespace(sb, colonEnd, true);
    private static int findNonWhitespace(AppendableCharSequence sb, int offset, boolean validateOWS) {
            } else if (validateOWS && !isOWS(c)) {
    public void testWhitespaceBeforeTransferEncoding01() {
                " Transfer-Encoding : chunked\r\n" +
    public void testWhitespaceBeforeTransferEncoding02() {
                " Transfer-Encoding : chunked\r\n" +
        assertTrue(channel.writeInbound(Unpooled.copiedBuffer(requestStr, CharsetUtil.US_ASCII)));
        assertTrue(request.decoderResult().cause() instanceof IllegalArgumentException);
Fixed code:         case 0x1c:
        case 0x1d:
        case 0x1e:
        case 0x1f:
        case 0x1c:
        case 0x1d:
        case 0x1e:
        case 0x1f:
        nameStart = findNonWhitespace(sb, 0);
        valueStart = findNonWhitespace(sb, colonEnd);
    private static int findNonWhitespace(AppendableCharSequence sb, int offset) {
            } else if (!isOWS(c)) {
import io.netty.buffer.ByteBuf;
    @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);
    }

    public void testWhitespaceInTransferEncoding01() {
                "Transfer-Encoding : chunked\r\n" +
    public void testWhitespaceInTransferEncoding02() {
                "Transfer-Encoding : chunked\r\n" +
        c.release();
        testInvalidHeaders0(Unpooled.copiedBuffer(requestStr, CharsetUtil.US_ASCII));
    }

    private static void testInvalidHeaders0(ByteBuf requestBuffer) {
        assertTrue(channel.writeInbound(requestBuffer));
        assertThat(request.decoderResult().cause(), instanceOf(IllegalArgumentException.class));

    @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());
    }