CVE-2021-21409: Inconsistent Interpretation of HTTP Requests ('HTTP Request/Response Smuggling') (CWE-444)
value: Netty is an open-source, asynchronous event-driven network application framework for rapid development of maintainable high performance protocol servers & clients. In Netty (io.netty:netty-codec-http2) before version 4.1.61.Final there is a vulnerability that enables request smuggling. The content-length header is not correctly validated if the request only uses a single Http2HeaderFrame with the endStream set to to true. This could lead to request smuggling if the request is proxied to a remote peer and translated to HTTP/1.1. This is a followup of GHSA-wm47-8v5p-wjpj/CVE-2021-21295 which did miss to fix this one case. This was fixed as part of 4.1.61.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: 5.9
Published: 2021-03-30T15:15Z
Repository: https://github.com/netty/netty
Language: Java
Files changed: codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java, codec-http2/src/test/java/io/netty/handler/codec/http2/Http2MultiplexTest.java
Commit date: 2021-03-30 09:40:47 +0200
Commit message: Merge pull request from GHSA-f256-j965-7f32

Motivation:

We also need to ensure that all the header validation is done when a single header with the endStream flag is received

Modifications:

- Adjust code to always enforce the validation
- Add more unit tests

Result:

Always correctly validate
Diff: diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java
index 7fe74de3c1..ac24992a10 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java
@@ -351,14 +351,17 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
         @Override
         public void onHeadersRead(ChannelHandlerContext ctx, int streamId, Http2Headers headers, int streamDependency,
                 short weight, boolean exclusive, int padding, boolean endOfStream) throws Http2Exception {
             Http2Stream stream = connection.stream(streamId);
             boolean allowHalfClosedRemote = false;
+            boolean isTrailers = false;
             if (stream == null && !connection.streamMayHaveExisted(streamId)) {
                 stream = connection.remote().createStream(streamId, endOfStream);
                 // Allow the state to be HALF_CLOSE_REMOTE if we're creating it in that state.
                 allowHalfClosedRemote = stream.state() == HALF_CLOSED_REMOTE;
+            } else if (stream != null) {
+                isTrailers = stream.isHeadersReceived();
             }
 
             if (shouldIgnoreHeadersOrDataFrame(ctx, streamId, stream, "HEADERS")) {
                 return;
             }
@@ -392,11 +395,11 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
                     // Connection error.
                     throw connectionError(PROTOCOL_ERROR, "Stream %d in unexpected state: %s", stream.id(),
                             stream.state());
             }
 
-            if (!stream.isHeadersReceived()) {
+            if (!isTrailers) {
                 // extract the content-length header
                 List<? extends CharSequence> contentLength = headers.getAll(HttpHeaderNames.CONTENT_LENGTH);
                 if (contentLength != null && !contentLength.isEmpty()) {
                     try {
                         long cLength = HttpUtil.normalizeAndGetContentLength(contentLength, false, true);
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2MultiplexTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2MultiplexTest.java
index a0e06788e2..3ed2057361 100644
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2MultiplexTest.java
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2MultiplexTest.java
@@ -222,14 +222,57 @@ public abstract class Http2MultiplexTest<C extends Http2FrameCodec> {
         assertNull(inboundHandler.readInbound());
     }
 
     @Test
     public void headerMultipleContentLengthValidationShouldPropagate() {
+        headerMultipleContentLengthValidationShouldPropagate(false);
+    }
+
+    @Test
+    public void headerMultipleContentLengthValidationShouldPropagateWithEndStream() {
+        headerMultipleContentLengthValidationShouldPropagate(true);
+    }
+
+    private void headerMultipleContentLengthValidationShouldPropagate(boolean endStream) {
         LastInboundHandler inboundHandler = new LastInboundHandler();
         request.addLong(HttpHeaderNames.CONTENT_LENGTH, 0);
         request.addLong(HttpHeaderNames.CONTENT_LENGTH, 1);
-        Http2StreamChannel channel = newInboundStream(3, false, inboundHandler);
+        Http2StreamChannel channel = newInboundStream(3, endStream, inboundHandler);
+        try {
+            inboundHandler.checkException();
+            fail();
+        } catch (Exception e) {
+            assertThat(e, CoreMatchers.<Exception>instanceOf(StreamException.class));
+        }
+        assertNull(inboundHandler.readInbound());
+        assertFalse(channel.isActive());
+    }
+
+    @Test
+    public void headerPlusSignContentLengthValidationShouldPropagate() {
+        headerSignContentLengthValidationShouldPropagateWithEndStream(false, false);
+    }
+
+    @Test
+    public void headerPlusSignContentLengthValidationShouldPropagateWithEndStream() {
+        headerSignContentLengthValidationShouldPropagateWithEndStream(false, true);
+    }
+
+    @Test
+    public void headerMinusSignContentLengthValidationShouldPropagate() {
+        headerSignContentLengthValidationShouldPropagateWithEndStream(true, false);
+    }
+
+    @Test
+    public void headerMinusSignContentLengthValidationShouldPropagateWithEndStream() {
+        headerSignContentLengthValidationShouldPropagateWithEndStream(true, true);
+    }
+
+    private void headerSignContentLengthValidationShouldPropagateWithEndStream(boolean minus, boolean endStream) {
+        LastInboundHandler inboundHandler = new LastInboundHandler();
+        request.add(HttpHeaderNames.CONTENT_LENGTH, (minus ? "-" : "+") + 1);
+        Http2StreamChannel channel = newInboundStream(3, endStream, inboundHandler);
         try {
             inboundHandler.checkException();
             fail();
         } catch (Exception e) {
             assertThat(e, CoreMatchers.<Exception>instanceOf(StreamException.class));

Vulnerable code:             if (!stream.isHeadersReceived()) {
        Http2StreamChannel channel = newInboundStream(3, false, inboundHandler);
Fixed code:             boolean isTrailers = false;
            } else if (stream != null) {
                isTrailers = stream.isHeadersReceived();
            if (!isTrailers) {
        headerMultipleContentLengthValidationShouldPropagate(false);
    }

    @Test
    public void headerMultipleContentLengthValidationShouldPropagateWithEndStream() {
        headerMultipleContentLengthValidationShouldPropagate(true);
    }

    private void headerMultipleContentLengthValidationShouldPropagate(boolean endStream) {
        Http2StreamChannel channel = newInboundStream(3, endStream, inboundHandler);
        try {
            inboundHandler.checkException();
            fail();
        } catch (Exception e) {
            assertThat(e, CoreMatchers.<Exception>instanceOf(StreamException.class));
        }
        assertNull(inboundHandler.readInbound());
        assertFalse(channel.isActive());
    }

    @Test
    public void headerPlusSignContentLengthValidationShouldPropagate() {
        headerSignContentLengthValidationShouldPropagateWithEndStream(false, false);
    }

    @Test
    public void headerPlusSignContentLengthValidationShouldPropagateWithEndStream() {
        headerSignContentLengthValidationShouldPropagateWithEndStream(false, true);
    }

    @Test
    public void headerMinusSignContentLengthValidationShouldPropagate() {
        headerSignContentLengthValidationShouldPropagateWithEndStream(true, false);
    }

    @Test
    public void headerMinusSignContentLengthValidationShouldPropagateWithEndStream() {
        headerSignContentLengthValidationShouldPropagateWithEndStream(true, true);
    }

    private void headerSignContentLengthValidationShouldPropagateWithEndStream(boolean minus, boolean endStream) {
        LastInboundHandler inboundHandler = new LastInboundHandler();
        request.add(HttpHeaderNames.CONTENT_LENGTH, (minus ? "-" : "+") + 1);
        Http2StreamChannel channel = newInboundStream(3, endStream, inboundHandler);