CVE-2021-21295: 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.60.Final there is a vulnerability that enables request smuggling. If a Content-Length header is present in the original HTTP/2 request, the field is not validated by `Http2MultiplexHandler` as it is propagated up. This is fine as long as the request is not proxied through as HTTP/1.1. If the request comes in as an HTTP/2 stream, gets converted into the HTTP/1.1 domain objects (`HttpRequest`, `HttpContent`, etc.) via `Http2StreamFrameToHttpObjectCodec `and then sent up to the child channel's pipeline and proxied through a remote peer as HTTP/1.1 this may result in request smuggling. In a proxy case, users may assume the content-length is validated somehow, which is not the case. If the request is forwarded to a backend channel that is a HTTP/1.1 connection, the Content-Length now has meaning and needs to be checked. An attacker can smuggle requests inside the body as it gets downgraded from HTTP/2 to HTTP/1.1. For an example attack refer to the linked GitHub Advisory. Users are only affected if all of this is true: `HTTP2MultiplexCodec` or `Http2FrameCodec` is used, `Http2StreamFrameToHttpObjectCodec` is used to convert to HTTP/1.1 objects, and these HTTP/1.1 objects are forwarded to another remote peer. This has been patched in 4.1.60.Final As a workaround, the user can do the validation by themselves by implementing a custom `ChannelInboundHandler` that is put in the `ChannelPipeline` behind `Http2StreamFrameToHttpObjectCodec`.
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-09T19:15Z
Repository: https://github.com/netty/netty
Language: Java
Files changed: codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java, codec-http/src/main/java/io/netty/handler/codec/http/HttpUtil.java, codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java, codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java
Commit date: 2021-03-09 08:20:09 +0100
Commit message: Merge pull request from GHSA-wm47-8v5p-wjpj

Motivation:

As stated by https://tools.ietf.org/html/rfc7540#section-8.1.2.6 we should report a stream error if the content-length does not match the sum of all data frames.

Modifications:

- Verify that the sum of data frames match if a content-length header was send.
- Handle multiple content-length headers and also handle negative values
- Add io.netty.http2.validateContentLength system property which allows to disable the more strict validation
- Add unit tests

Result:

Correctly handle the case when the content-length header was included but not match what is send and also when content-length header is invalid
Diff: 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 db4904407d..761041aed5 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
@@ -14,11 +14,10 @@
  * under the License.
  */
 package io.netty.handler.codec.http;
 
 import static io.netty.util.internal.ObjectUtil.checkPositive;
-import static io.netty.util.internal.StringUtil.COMMA;
 
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.Unpooled;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.ChannelPipeline;
@@ -628,53 +627,20 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
         // reset name and value fields
         name = null;
         value = null;
 
         List<String> contentLengthFields = headers.getAll(HttpHeaderNames.CONTENT_LENGTH);
-
         if (!contentLengthFields.isEmpty()) {
+            HttpVersion version = message.protocolVersion();
+            boolean isHttp10OrEarlier = version.majorVersion() < 1 || (version.majorVersion() == 1
+                    && version.minorVersion() == 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.
-            boolean multipleContentLengths =
-                    contentLengthFields.size() > 1 || contentLengthFields.get(0).indexOf(COMMA) >= 0;
-            if (multipleContentLengths && message.protocolVersion() == HttpVersion.HTTP_1_1) {
-                if (allowDuplicateContentLengths) {
-                    // Find and enforce that all Content-Length values are the same
-                    String firstValue = null;
-                    for (String field : contentLengthFields) {
-                        String[] tokens = COMMA_PATTERN.split(field, -1);
-                        for (String token : tokens) {
-                            String trimmed = token.trim();
-                            if (firstValue == null) {
-                                firstValue = trimmed;
-                            } else if (!trimmed.equals(firstValue)) {
-                                throw new IllegalArgumentException(
-                                        "Multiple Content-Length values found: " + contentLengthFields);
-                            }
-                        }
-                    }
-                    // Replace the duplicated field-values with a single valid Content-Length field
-                    headers.set(HttpHeaderNames.CONTENT_LENGTH, firstValue);
-                    contentLength = Long.parseLong(firstValue);
-                } else {
-                    // Reject the message as invalid
-                    throw new IllegalArgumentException(
-                            "Multiple Content-Length values found: " + contentLengthFields);
-                }
-            } else {
-                contentLength = Long.parseLong(contentLengthFields.get(0));
+            contentLength = HttpUtil.normalizeAndGetContentLength(contentLengthFields,
+                    isHttp10OrEarlier, allowDuplicateContentLengths);
+            if (contentLength != -1) {
+                headers.set(HttpHeaderNames.CONTENT_LENGTH, contentLength);
             }
         }
 
         if (isContentAlwaysEmpty(message)) {
             HttpUtil.setTransferEncodingChunked(message, false);
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpUtil.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpUtil.java
index 8b9aedc551..49ed25510f 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpUtil.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpUtil.java
@@ -22,22 +22,27 @@ import java.nio.charset.IllegalCharsetNameException;
 import java.nio.charset.UnsupportedCharsetException;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 
+import io.netty.handler.codec.Headers;
 import io.netty.util.AsciiString;
 import io.netty.util.CharsetUtil;
 import io.netty.util.NetUtil;
 import io.netty.util.internal.ObjectUtil;
+import io.netty.util.internal.UnstableApi;
+
+import static io.netty.util.internal.StringUtil.COMMA;
 
 /**
  * Utility methods useful in the HTTP context.
  */
 public final class HttpUtil {
 
     private static final AsciiString CHARSET_EQUALS = AsciiString.of(HttpHeaderValues.CHARSET + "=");
     private static final AsciiString SEMICOLON = AsciiString.cached(";");
+    private static final String COMMA_STRING = String.valueOf(COMMA);
 
     private HttpUtil() { }
 
     /**
      * Determine if a uri is in origin-form according to
@@ -528,6 +533,87 @@ public final class HttpUtil {
             }
             return '[' + hostString + ']';
         }
         return hostString;
     }
+
+    /**
+     * Validates, and optionally extracts the content length from headers. This method is not intended for
+     * general use, but is here to be shared between HTTP/1 and HTTP/2 parsing.
+     *
+     * @param contentLengthFields the content-length header fields.
+     * @param isHttp10OrEarlier {@code true} if we are handling HTTP/1.0 or earlier
+     * @param allowDuplicateContentLengths {@code true}  if multiple, identical-value content lengths should be allowed.
+     * @return the normalized content length from the headers or {@code -1} if the fields were empty.
+     * @throws IllegalArgumentException if the content-length fields are not valid
+     */
+    @UnstableApi
+    public static long normalizeAndGetContentLength(
+            List<? extends CharSequence> contentLengthFields, boolean isHttp10OrEarlier,
+            boolean allowDuplicateContentLengths) {
+        if (contentLengthFields.isEmpty()) {
+            return -1;
+        }
+
+        // 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.
+        String firstField = contentLengthFields.get(0).toString();
+        boolean multipleContentLengths =
+                contentLengthFields.size() > 1 || firstField.indexOf(COMMA) >= 0;
+
+        if (multipleContentLengths && !isHttp10OrEarlier) {
+            if (allowDuplicateContentLengths) {
+                // Find and enforce that all Content-Length values are the same
+                String firstValue = null;
+                for (CharSequence field : contentLengthFields) {
+                    String[] tokens = field.toString().split(COMMA_STRING, -1);
+                    for (String token : tokens) {
+                        String trimmed = token.trim();
+                        if (firstValue == null) {
+                            firstValue = trimmed;
+                        } else if (!trimmed.equals(firstValue)) {
+                            throw new IllegalArgumentException(
+                                    "Multiple Content-Length values found: " + contentLengthFields);
+                        }
+                    }
+                }
+                // Replace the duplicated field-values with a single valid Content-Length field
+                firstField = firstValue;
+            } else {
+                // Reject the message as invalid
+                throw new IllegalArgumentException(
+                        "Multiple Content-Length values found: " + contentLengthFields);
+            }
+        }
+        // Ensure we not allow sign as part of the content-length:
+        // See https://github.com/squid-cache/squid/security/advisories/GHSA-qf3v-rc95-96j5
+        if (!Character.isDigit(firstField.charAt(0))) {
+            // Reject the message as invalid
+            throw new IllegalArgumentException(
+                    "Content-Length value is not a number: " + firstField);
+        }
+        try {
+            final long value = Long.parseLong(firstField);
+            if (value < 0) {
+                // Reject the message as invalid
+                throw new IllegalArgumentException(
+                        "Content-Length value must be >=0: " + value);
+            }
+            return value;
+        } catch (NumberFormatException e) {
+            // Reject the message as invalid
+            throw new IllegalArgumentException(
+                    "Content-Length value is not a number: " + firstField, e);
+        }
+    }
 }
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 1bbaf632b5..9655d6bc18 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
@@ -14,12 +14,15 @@
  */
 package io.netty.handler.codec.http2;
 
 import io.netty.buffer.ByteBuf;
 import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.http.HttpHeaderNames;
 import io.netty.handler.codec.http.HttpStatusClass;
+import io.netty.handler.codec.http.HttpUtil;
 import io.netty.handler.codec.http2.Http2Connection.Endpoint;
+import io.netty.util.internal.SystemPropertyUtil;
 import io.netty.util.internal.UnstableApi;
 import io.netty.util.internal.logging.InternalLogger;
 import io.netty.util.internal.logging.InternalLoggerFactory;
 
 import java.util.List;
@@ -47,20 +50,23 @@ import static java.lang.Math.min;
  * This interface enforces inbound flow control functionality through
  * {@link Http2LocalFlowController}
  */
 @UnstableApi
 public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
+    private static final boolean VALIDATE_CONTENT_LENGTH =
+            SystemPropertyUtil.getBoolean("io.netty.http2.validateContentLength", true);
     private static final InternalLogger logger = InternalLoggerFactory.getInstance(DefaultHttp2ConnectionDecoder.class);
     private Http2FrameListener internalFrameListener = new PrefaceFrameListener();
     private final Http2Connection connection;
     private Http2LifecycleManager lifecycleManager;
     private final Http2ConnectionEncoder encoder;
     private final Http2FrameReader frameReader;
     private Http2FrameListener listener;
     private final Http2PromisedRequestVerifier requestVerifier;
     private final Http2SettingsReceivedConsumer settingsReceivedConsumer;
     private final boolean autoAckPing;
+    private final Http2Connection.PropertyKey contentLengthKey;
 
     public DefaultHttp2ConnectionDecoder(Http2Connection connection,
                                          Http2ConnectionEncoder encoder,
                                          Http2FrameReader frameReader) {
         this(connection, encoder, frameReader, ALWAYS_VERIFY);
@@ -123,10 +129,11 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
                         Http2SettingsReceivedConsumer.class);
             }
             settingsReceivedConsumer = (Http2SettingsReceivedConsumer) encoder;
         }
         this.connection = checkNotNull(connection, "connection");
+        contentLengthKey = this.connection.newKey();
         this.frameReader = checkNotNull(frameReader, "frameReader");
         this.encoder = checkNotNull(encoder, "encoder");
         this.requestVerifier = checkNotNull(requestVerifier, "requestVerifier");
         if (connection.local().flowController() == null) {
             connection.local().flowController(new DefaultHttp2LocalFlowController(connection));
@@ -221,20 +228,38 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
     void onUnknownFrame0(ChannelHandlerContext ctx, byte frameType, int streamId, Http2Flags flags,
             ByteBuf payload) throws Http2Exception {
         listener.onUnknownFrame(ctx, frameType, streamId, flags, payload);
     }
 
+    // See https://tools.ietf.org/html/rfc7540#section-8.1.2.6
+    private void verifyContentLength(Http2Stream stream, int data, boolean isEnd) throws Http2Exception {
+        if (!VALIDATE_CONTENT_LENGTH) {
+            return;
+        }
+        ContentLength contentLength = stream.getProperty(contentLengthKey);
+        if (contentLength != null) {
+            try {
+                contentLength.increaseReceivedBytes(connection.isServer(), stream.id(), data, isEnd);
+            } finally {
+                if (isEnd) {
+                    stream.removeProperty(contentLengthKey);
+                }
+            }
+        }
+    }
+
     /**
      * Handles all inbound frames from the network.
      */
     private final class FrameReadListener implements Http2FrameListener {
         @Override
         public int onDataRead(final ChannelHandlerContext ctx, int streamId, ByteBuf data, int padding,
                               boolean endOfStream) throws Http2Exception {
             Http2Stream stream = connection.stream(streamId);
             Http2LocalFlowController flowController = flowController();
-            int bytesToReturn = data.readableBytes() + padding;
+            int readable = data.readableBytes();
+            int bytesToReturn = readable + padding;
 
             final boolean shouldIgnore;
             try {
                 shouldIgnore = shouldIgnoreHeadersOrDataFrame(ctx, streamId, stream, "DATA");
             } catch (Http2Exception e) {
@@ -257,11 +282,10 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
                 verifyStreamMayHaveExisted(streamId);
 
                 // All bytes have been consumed.
                 return bytesToReturn;
             }
-
             Http2Exception error = null;
             switch (stream.state()) {
                 case OPEN:
                 case HALF_CLOSED_LOCAL:
                     break;
@@ -285,10 +309,12 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
                 // If the stream is in an invalid state to receive the frame, throw the error.
                 if (error != null) {
                     throw error;
                 }
 
+                verifyContentLength(stream, readable, endOfStream);
+
                 // Call back the application and retrieve the number of bytes that have been
                 // immediately processed.
                 bytesToReturn = listener.onDataRead(ctx, streamId, data, padding, endOfStream);
                 return bytesToReturn;
             } catch (Http2Exception e) {
@@ -365,18 +391,38 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
                     // Connection error.
                     throw connectionError(PROTOCOL_ERROR, "Stream %d in unexpected state: %s", stream.id(),
                             stream.state());
             }
 
-            stream.headersReceived(isInformational);
-            encoder.flowController().updateDependencyTree(streamId, streamDependency, weight, exclusive);
-
-            listener.onHeadersRead(ctx, streamId, headers, streamDependency, weight, exclusive, padding, endOfStream);
+            if (!stream.isHeadersReceived()) {
+                // 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);
+                        if (cLength != -1) {
+                            headers.setLong(HttpHeaderNames.CONTENT_LENGTH, cLength);
+                            stream.setProperty(contentLengthKey, new ContentLength(cLength));
+                        }
+                    } catch (IllegalArgumentException e) {
+                        throw streamError(stream.id(), PROTOCOL_ERROR,
+                                "Multiple content-length headers received", e);
+                    }
+                }
+            }
 
-            // If the headers completes this stream, close it.
-            if (endOfStream) {
-                lifecycleManager.closeStreamRemote(stream, ctx.newSucceededFuture());
+            stream.headersReceived(isInformational);
+            try {
+                verifyContentLength(stream, 0, endOfStream);
+                encoder.flowController().updateDependencyTree(streamId, streamDependency, weight, exclusive);
+                listener.onHeadersRead(ctx, streamId, headers, streamDependency,
+                        weight, exclusive, padding, endOfStream);
+            } finally {
+                // If the headers completes this stream, close it.
+                if (endOfStream) {
+                    lifecycleManager.closeStreamRemote(stream, ctx.newSucceededFuture());
+                }
             }
         }
 
         @Override
         public void onPriorityRead(ChannelHandlerContext ctx, int streamId, int streamDependency, short weight,
@@ -734,6 +780,42 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
         public void onUnknownFrame(ChannelHandlerContext ctx, byte frameType, int streamId, Http2Flags flags,
                 ByteBuf payload) throws Http2Exception {
             onUnknownFrame0(ctx, frameType, streamId, flags, payload);
         }
     }
+
+    private static final class ContentLength {
+        private final long expected;
+        private long seen;
+
+        ContentLength(long expected) {
+            this.expected = expected;
+        }
+
+        void increaseReceivedBytes(boolean server, int streamId, int bytes, boolean isEnd) throws Http2Exception {
+            seen += bytes;
+            // Check for overflow
+            if (seen < 0) {
+                throw streamError(streamId, PROTOCOL_ERROR,
+                        "Received amount of data did overflow and so not match content-length header %d", expected);
+            }
+            // Check if we received more data then what was advertised via the content-length header.
+            if (seen > expected) {
+                throw streamError(streamId, PROTOCOL_ERROR,
+                        "Received amount of data %d does not match content-length header %d", seen, expected);
+            }
+
+            if (isEnd) {
+                if (seen == 0 && !server) {
+                    // This may be a response to a HEAD request, let's just allow it.
+                    return;
+                }
+
+                // Check that we really saw what was told via the content-length header.
+                if (expected > seen) {
+                    throw streamError(streamId, PROTOCOL_ERROR,
+                            "Received amount of data %d does not match content-length header %d", seen, expected);
+                }
+            }
+        }
+    }
 }
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java
index 2f91a58d78..42ab7031a1 100644
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java
@@ -19,21 +19,25 @@ import io.netty.buffer.UnpooledByteBufAllocator;
 import io.netty.channel.Channel;
 import io.netty.channel.ChannelFuture;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.ChannelPromise;
 import io.netty.channel.DefaultChannelPromise;
+import io.netty.handler.codec.http.HttpHeaderNames;
 import io.netty.handler.codec.http.HttpResponseStatus;
 import junit.framework.AssertionFailedError;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
+import org.mockito.ArgumentMatchers;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 
 import java.util.Collections;
+import java.util.IdentityHashMap;
+import java.util.Map;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import static io.netty.buffer.Unpooled.EMPTY_BUFFER;
 import static io.netty.buffer.Unpooled.wrappedBuffer;
 import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_PRIORITY_WEIGHT;
@@ -132,10 +136,25 @@ public class DefaultHttp2ConnectionDecoderTest {
         final AtomicInteger headersReceivedState = new AtomicInteger();
         when(channel.isActive()).thenReturn(true);
         when(stream.id()).thenReturn(STREAM_ID);
         when(stream.state()).thenReturn(OPEN);
         when(stream.open(anyBoolean())).thenReturn(stream);
+
+        final Map<Object, Object> properties = new IdentityHashMap<Object, Object>();
+        when(stream.getProperty(ArgumentMatchers.<Http2Connection.PropertyKey>any())).thenAnswer(new Answer<Object>() {
+            @Override
+            public Object answer(InvocationOnMock invocationOnMock) {
+                return properties.get(invocationOnMock.getArgument(0));
+            }
+        });
+        when(stream.setProperty(ArgumentMatchers.<Http2Connection.PropertyKey>any(), any())).then(new Answer<Object>() {
+            @Override
+            public Object answer(InvocationOnMock invocationOnMock) {
+                return properties.put(invocationOnMock.getArgument(0), invocationOnMock.getArgument(1));
+            }
+        });
+
         when(pushStream.id()).thenReturn(PUSH_STREAM_ID);
         doAnswer(new Answer<Boolean>() {
             @Override
             public Boolean answer(InvocationOnMock in) throws Throwable {
                 return (headersReceivedState.get() & STATE_RECV_HEADERS) != 0;
@@ -772,10 +791,119 @@ public class DefaultHttp2ConnectionDecoderTest {
         decode().onGoAwayRead(ctx, 1, 2L, EMPTY_BUFFER);
         verify(connection).goAwayReceived(eq(1), eq(2L), eq(EMPTY_BUFFER));
         verify(listener).onGoAwayRead(eq(ctx), eq(1), eq(2L), eq(EMPTY_BUFFER));
     }
 
+    @Test(expected = Http2Exception.StreamException.class)
+    public void dataContentLengthMissmatch() throws Exception {
+        dataContentLengthInvalid(false);
+    }
+
+    @Test(expected = Http2Exception.StreamException.class)
+    public void dataContentLengthInvalid() throws Exception {
+        dataContentLengthInvalid(true);
+    }
+
+    private void dataContentLengthInvalid(boolean negative) throws Exception {
+        final ByteBuf data = dummyData();
+        int padding = 10;
+        int processedBytes = data.readableBytes() + padding;
+        mockFlowControl(processedBytes);
+        try {
+            decode().onHeadersRead(ctx, STREAM_ID, new DefaultHttp2Headers()
+                    .setLong(HttpHeaderNames.CONTENT_LENGTH, negative ? -1L : 1L), padding, false);
+            decode().onDataRead(ctx, STREAM_ID, data, padding, true);
+            verify(localFlow).receiveFlowControlledFrame(eq(stream), eq(data), eq(padding), eq(true));
+            verify(localFlow).consumeBytes(eq(stream), eq(processedBytes));
+
+            verify(listener, times(1)).onHeadersRead(eq(ctx), anyInt(),
+                    any(Http2Headers.class), eq(0), eq(DEFAULT_PRIORITY_WEIGHT), eq(false),
+                    eq(padding), eq(false));
+            // Verify that the event was absorbed and not propagated to the observer.
+            verify(listener, never()).onDataRead(eq(ctx), anyInt(), any(ByteBuf.class), anyInt(), anyBoolean());
+        } finally {
+            data.release();
+        }
+    }
+
+    @Test(expected = Http2Exception.StreamException.class)
+    public void headersContentLengthPositiveSign() throws Exception {
+        headersContentLengthSign("+1");
+    }
+
+    @Test(expected = Http2Exception.StreamException.class)
+    public void headersContentLengthNegativeSign() throws Exception {
+        headersContentLengthSign("-1");
+    }
+
+    private void headersContentLengthSign(String length) throws Exception {
+        int padding = 10;
+        when(connection.isServer()).thenReturn(true);
+        decode().onHeadersRead(ctx, STREAM_ID, new DefaultHttp2Headers()
+                .set(HttpHeaderNames.CONTENT_LENGTH, length), padding, false);
+
+        // Verify that the event was absorbed and not propagated to the observer.
+        verify(listener, never()).onHeadersRead(eq(ctx), anyInt(),
+                any(Http2Headers.class), anyInt(), anyShort(), anyBoolean(), anyInt(), anyBoolean());
+    }
+
+    @Test(expected = Http2Exception.StreamException.class)
+    public void headersContentLengthMissmatch() throws Exception {
+        headersContentLength(false);
+    }
+
+    @Test(expected = Http2Exception.StreamException.class)
+    public void headersContentLengthInvalid() throws Exception {
+        headersContentLength(true);
+    }
+
+    private void headersContentLength(boolean negative) throws Exception {
+        int padding = 10;
+        when(connection.isServer()).thenReturn(true);
+        decode().onHeadersRead(ctx, STREAM_ID, new DefaultHttp2Headers()
+                .setLong(HttpHeaderNames.CONTENT_LENGTH, negative ? -1L : 1L), padding, true);
+
+        // Verify that the event was absorbed and not propagated to the observer.
+        verify(listener, never()).onHeadersRead(eq(ctx), anyInt(),
+                any(Http2Headers.class), anyInt(), anyShort(), anyBoolean(), anyInt(), anyBoolean());
+    }
+
+    @Test
+    public void multipleHeadersContentLengthSame() throws Exception {
+        multipleHeadersContentLength(true);
+    }
+
+    @Test(expected = Http2Exception.StreamException.class)
+    public void multipleHeadersContentLengthDifferent() throws Exception {
+        multipleHeadersContentLength(false);
+    }
+
+    private void multipleHeadersContentLength(boolean same) throws Exception {
+        int padding = 10;
+        when(connection.isServer()).thenReturn(true);
+        Http2Headers headers = new DefaultHttp2Headers();
+        if (same) {
+            headers.addLong(HttpHeaderNames.CONTENT_LENGTH, 0);
+            headers.addLong(HttpHeaderNames.CONTENT_LENGTH, 0);
+        } else {
+            headers.addLong(HttpHeaderNames.CONTENT_LENGTH, 0);
+            headers.addLong(HttpHeaderNames.CONTENT_LENGTH, 1);
+        }
+
+        decode().onHeadersRead(ctx, STREAM_ID, headers, padding, true);
+
+        if (same) {
+            verify(listener, times(1)).onHeadersRead(eq(ctx), anyInt(),
+                    any(Http2Headers.class), anyInt(), anyShort(), anyBoolean(), anyInt(), anyBoolean());
+            assertEquals(1, headers.getAll(HttpHeaderNames.CONTENT_LENGTH).size());
+        } else {
+            // Verify that the event was absorbed and not propagated to the observer.
+            verify(listener, never()).onHeadersRead(eq(ctx), anyInt(),
+                    any(Http2Headers.class), anyInt(), anyShort(), anyBoolean(), anyInt(), anyBoolean());
+        }
+    }
+
     private static ByteBuf dummyData() {
         // The buffer is purposely 8 bytes so it will even work for a ping frame.
         return wrappedBuffer("abcdefgh".getBytes(UTF_8));
     }
 

Vulnerable code: import static io.netty.util.internal.StringUtil.COMMA;

            //
            // 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.
            boolean multipleContentLengths =
                    contentLengthFields.size() > 1 || contentLengthFields.get(0).indexOf(COMMA) >= 0;
            if (multipleContentLengths && message.protocolVersion() == HttpVersion.HTTP_1_1) {
                if (allowDuplicateContentLengths) {
                    // Find and enforce that all Content-Length values are the same
                    String firstValue = null;
                    for (String field : contentLengthFields) {
                        String[] tokens = COMMA_PATTERN.split(field, -1);
                        for (String token : tokens) {
                            String trimmed = token.trim();
                            if (firstValue == null) {
                                firstValue = trimmed;
                            } else if (!trimmed.equals(firstValue)) {
                                throw new IllegalArgumentException(
                                        "Multiple Content-Length values found: " + contentLengthFields);
                            }
                        }
                    }
                    // Replace the duplicated field-values with a single valid Content-Length field
                    headers.set(HttpHeaderNames.CONTENT_LENGTH, firstValue);
                    contentLength = Long.parseLong(firstValue);
                } else {
                    // Reject the message as invalid
                    throw new IllegalArgumentException(
                            "Multiple Content-Length values found: " + contentLengthFields);
                }
            } else {
                contentLength = Long.parseLong(contentLengthFields.get(0));
            int bytesToReturn = data.readableBytes() + padding;

            stream.headersReceived(isInformational);
            encoder.flowController().updateDependencyTree(streamId, streamDependency, weight, exclusive);

            listener.onHeadersRead(ctx, streamId, headers, streamDependency, weight, exclusive, padding, endOfStream);
            // If the headers completes this stream, close it.
            if (endOfStream) {
                lifecycleManager.closeStreamRemote(stream, ctx.newSucceededFuture());
Fixed code:             HttpVersion version = message.protocolVersion();
            boolean isHttp10OrEarlier = version.majorVersion() < 1 || (version.majorVersion() == 1
                    && version.minorVersion() == 0);
            contentLength = HttpUtil.normalizeAndGetContentLength(contentLengthFields,
                    isHttp10OrEarlier, allowDuplicateContentLengths);
            if (contentLength != -1) {
                headers.set(HttpHeaderNames.CONTENT_LENGTH, contentLength);
import io.netty.handler.codec.Headers;
import io.netty.util.internal.UnstableApi;

import static io.netty.util.internal.StringUtil.COMMA;
    private static final String COMMA_STRING = String.valueOf(COMMA);

    /**
     * Validates, and optionally extracts the content length from headers. This method is not intended for
     * general use, but is here to be shared between HTTP/1 and HTTP/2 parsing.
     *
     * @param contentLengthFields the content-length header fields.
     * @param isHttp10OrEarlier {@code true} if we are handling HTTP/1.0 or earlier
     * @param allowDuplicateContentLengths {@code true}  if multiple, identical-value content lengths should be allowed.
     * @return the normalized content length from the headers or {@code -1} if the fields were empty.
     * @throws IllegalArgumentException if the content-length fields are not valid
     */
    @UnstableApi
    public static long normalizeAndGetContentLength(
            List<? extends CharSequence> contentLengthFields, boolean isHttp10OrEarlier,
            boolean allowDuplicateContentLengths) {
        if (contentLengthFields.isEmpty()) {
            return -1;
        }

        // 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.
        String firstField = contentLengthFields.get(0).toString();
        boolean multipleContentLengths =
                contentLengthFields.size() > 1 || firstField.indexOf(COMMA) >= 0;

        if (multipleContentLengths && !isHttp10OrEarlier) {
            if (allowDuplicateContentLengths) {
                // Find and enforce that all Content-Length values are the same
                String firstValue = null;
                for (CharSequence field : contentLengthFields) {
                    String[] tokens = field.toString().split(COMMA_STRING, -1);
                    for (String token : tokens) {
                        String trimmed = token.trim();
                        if (firstValue == null) {
                            firstValue = trimmed;
                        } else if (!trimmed.equals(firstValue)) {
                            throw new IllegalArgumentException(
                                    "Multiple Content-Length values found: " + contentLengthFields);
                        }
                    }
                }
                // Replace the duplicated field-values with a single valid Content-Length field
                firstField = firstValue;
            } else {
                // Reject the message as invalid
                throw new IllegalArgumentException(
                        "Multiple Content-Length values found: " + contentLengthFields);
            }
        }
        // Ensure we not allow sign as part of the content-length:
        // See https://github.com/squid-cache/squid/security/advisories/GHSA-qf3v-rc95-96j5
        if (!Character.isDigit(firstField.charAt(0))) {
            // Reject the message as invalid
            throw new IllegalArgumentException(
                    "Content-Length value is not a number: " + firstField);
        }
        try {
            final long value = Long.parseLong(firstField);
            if (value < 0) {
                // Reject the message as invalid
                throw new IllegalArgumentException(
                        "Content-Length value must be >=0: " + value);
            }
            return value;
        } catch (NumberFormatException e) {
            // Reject the message as invalid
            throw new IllegalArgumentException(
                    "Content-Length value is not a number: " + firstField, e);
        }
    }
import io.netty.handler.codec.http.HttpHeaderNames;
import io.netty.handler.codec.http.HttpUtil;
import io.netty.util.internal.SystemPropertyUtil;
    private static final boolean VALIDATE_CONTENT_LENGTH =
            SystemPropertyUtil.getBoolean("io.netty.http2.validateContentLength", true);
    private final Http2Connection.PropertyKey contentLengthKey;
        contentLengthKey = this.connection.newKey();
    // See https://tools.ietf.org/html/rfc7540#section-8.1.2.6
    private void verifyContentLength(Http2Stream stream, int data, boolean isEnd) throws Http2Exception {
        if (!VALIDATE_CONTENT_LENGTH) {
            return;
        }
        ContentLength contentLength = stream.getProperty(contentLengthKey);
        if (contentLength != null) {
            try {
                contentLength.increaseReceivedBytes(connection.isServer(), stream.id(), data, isEnd);
            } finally {
                if (isEnd) {
                    stream.removeProperty(contentLengthKey);
                }
            }
        }
    }

            int readable = data.readableBytes();
            int bytesToReturn = readable + padding;
                verifyContentLength(stream, readable, endOfStream);

            if (!stream.isHeadersReceived()) {
                // 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);
                        if (cLength != -1) {
                            headers.setLong(HttpHeaderNames.CONTENT_LENGTH, cLength);
                            stream.setProperty(contentLengthKey, new ContentLength(cLength));
                        }
                    } catch (IllegalArgumentException e) {
                        throw streamError(stream.id(), PROTOCOL_ERROR,
                                "Multiple content-length headers received", e);
                    }
                }
            }
            stream.headersReceived(isInformational);
            try {
                verifyContentLength(stream, 0, endOfStream);
                encoder.flowController().updateDependencyTree(streamId, streamDependency, weight, exclusive);
                listener.onHeadersRead(ctx, streamId, headers, streamDependency,
                        weight, exclusive, padding, endOfStream);
            } finally {
                // If the headers completes this stream, close it.
                if (endOfStream) {
                    lifecycleManager.closeStreamRemote(stream, ctx.newSucceededFuture());
                }

    private static final class ContentLength {
        private final long expected;
        private long seen;

        ContentLength(long expected) {
            this.expected = expected;
        }

        void increaseReceivedBytes(boolean server, int streamId, int bytes, boolean isEnd) throws Http2Exception {
            seen += bytes;
            // Check for overflow
            if (seen < 0) {
                throw streamError(streamId, PROTOCOL_ERROR,
                        "Received amount of data did overflow and so not match content-length header %d", expected);
            }
            // Check if we received more data then what was advertised via the content-length header.
            if (seen > expected) {
                throw streamError(streamId, PROTOCOL_ERROR,
                        "Received amount of data %d does not match content-length header %d", seen, expected);
            }

            if (isEnd) {
                if (seen == 0 && !server) {
                    // This may be a response to a HEAD request, let's just allow it.
                    return;
                }

                // Check that we really saw what was told via the content-length header.
                if (expected > seen) {
                    throw streamError(streamId, PROTOCOL_ERROR,
                            "Received amount of data %d does not match content-length header %d", seen, expected);
                }
            }
        }
    }
import io.netty.handler.codec.http.HttpHeaderNames;
import org.mockito.ArgumentMatchers;
import java.util.IdentityHashMap;
import java.util.Map;

        final Map<Object, Object> properties = new IdentityHashMap<Object, Object>();
        when(stream.getProperty(ArgumentMatchers.<Http2Connection.PropertyKey>any())).thenAnswer(new Answer<Object>() {
            @Override
            public Object answer(InvocationOnMock invocationOnMock) {
                return properties.get(invocationOnMock.getArgument(0));
            }
        });
        when(stream.setProperty(ArgumentMatchers.<Http2Connection.PropertyKey>any(), any())).then(new Answer<Object>() {
            @Override
            public Object answer(InvocationOnMock invocationOnMock) {
                return properties.put(invocationOnMock.getArgument(0), invocationOnMock.getArgument(1));
            }
        });

    @Test(expected = Http2Exception.StreamException.class)
    public void dataContentLengthMissmatch() throws Exception {
        dataContentLengthInvalid(false);
    }

    @Test(expected = Http2Exception.StreamException.class)
    public void dataContentLengthInvalid() throws Exception {
        dataContentLengthInvalid(true);
    }

    private void dataContentLengthInvalid(boolean negative) throws Exception {
        final ByteBuf data = dummyData();
        int padding = 10;
        int processedBytes = data.readableBytes() + padding;
        mockFlowControl(processedBytes);
        try {
            decode().onHeadersRead(ctx, STREAM_ID, new DefaultHttp2Headers()
                    .setLong(HttpHeaderNames.CONTENT_LENGTH, negative ? -1L : 1L), padding, false);
            decode().onDataRead(ctx, STREAM_ID, data, padding, true);
            verify(localFlow).receiveFlowControlledFrame(eq(stream), eq(data), eq(padding), eq(true));
            verify(localFlow).consumeBytes(eq(stream), eq(processedBytes));

            verify(listener, times(1)).onHeadersRead(eq(ctx), anyInt(),
                    any(Http2Headers.class), eq(0), eq(DEFAULT_PRIORITY_WEIGHT), eq(false),
                    eq(padding), eq(false));
            // Verify that the event was absorbed and not propagated to the observer.
            verify(listener, never()).onDataRead(eq(ctx), anyInt(), any(ByteBuf.class), anyInt(), anyBoolean());
        } finally {
            data.release();
        }
    }

    @Test(expected = Http2Exception.StreamException.class)
    public void headersContentLengthPositiveSign() throws Exception {
        headersContentLengthSign("+1");
    }

    @Test(expected = Http2Exception.StreamException.class)
    public void headersContentLengthNegativeSign() throws Exception {
        headersContentLengthSign("-1");
    }

    private void headersContentLengthSign(String length) throws Exception {
        int padding = 10;
        when(connection.isServer()).thenReturn(true);
        decode().onHeadersRead(ctx, STREAM_ID, new DefaultHttp2Headers()
                .set(HttpHeaderNames.CONTENT_LENGTH, length), padding, false);

        // Verify that the event was absorbed and not propagated to the observer.
        verify(listener, never()).onHeadersRead(eq(ctx), anyInt(),
                any(Http2Headers.class), anyInt(), anyShort(), anyBoolean(), anyInt(), anyBoolean());
    }

    @Test(expected = Http2Exception.StreamException.class)
    public void headersContentLengthMissmatch() throws Exception {
        headersContentLength(false);
    }

    @Test(expected = Http2Exception.StreamException.class)
    public void headersContentLengthInvalid() throws Exception {
        headersContentLength(true);
    }

    private void headersContentLength(boolean negative) throws Exception {
        int padding = 10;
        when(connection.isServer()).thenReturn(true);
        decode().onHeadersRead(ctx, STREAM_ID, new DefaultHttp2Headers()
                .setLong(HttpHeaderNames.CONTENT_LENGTH, negative ? -1L : 1L), padding, true);

        // Verify that the event was absorbed and not propagated to the observer.
        verify(listener, never()).onHeadersRead(eq(ctx), anyInt(),
                any(Http2Headers.class), anyInt(), anyShort(), anyBoolean(), anyInt(), anyBoolean());
    }

    @Test
    public void multipleHeadersContentLengthSame() throws Exception {
        multipleHeadersContentLength(true);
    }

    @Test(expected = Http2Exception.StreamException.class)
    public void multipleHeadersContentLengthDifferent() throws Exception {
        multipleHeadersContentLength(false);
    }

    private void multipleHeadersContentLength(boolean same) throws Exception {
        int padding = 10;
        when(connection.isServer()).thenReturn(true);
        Http2Headers headers = new DefaultHttp2Headers();
        if (same) {
            headers.addLong(HttpHeaderNames.CONTENT_LENGTH, 0);
            headers.addLong(HttpHeaderNames.CONTENT_LENGTH, 0);
        } else {
            headers.addLong(HttpHeaderNames.CONTENT_LENGTH, 0);
            headers.addLong(HttpHeaderNames.CONTENT_LENGTH, 1);
        }

        decode().onHeadersRead(ctx, STREAM_ID, headers, padding, true);

        if (same) {
            verify(listener, times(1)).onHeadersRead(eq(ctx), anyInt(),
                    any(Http2Headers.class), anyInt(), anyShort(), anyBoolean(), anyInt(), anyBoolean());
            assertEquals(1, headers.getAll(HttpHeaderNames.CONTENT_LENGTH).size());
        } else {
            // Verify that the event was absorbed and not propagated to the observer.
            verify(listener, never()).onHeadersRead(eq(ctx), anyInt(),
                    any(Http2Headers.class), anyInt(), anyShort(), anyBoolean(), anyInt(), anyBoolean());
        }
    }