Feature/strict crlf for chunked transfer coding

.github/workflows/maven.yml

@@ -32,9 +32,13 @@ jobs:
       with:
         distribution: 'temurin'
         java-version: ${{ matrix.java_version }}
+    - name: Install Maven
+      run: |
+        curl -o ./apache-maven-3.9.9-bin.tar.gz https://dlcdn.apache.org/maven/maven-3/3.9.9/binaries/apache-maven-3.9.9-bin.tar.gz
+        tar -xf ./apache-maven-3.9.9-bin.tar.gz
     - name: Maven Build
       run: |
-        mvn --show-version \
+        ./apache-maven-3.9.9/bin/mvn --show-version \
             --no-transfer-progress \
             --activate-profiles staging \
             --define skipTests=true \
@@ -57,9 +61,13 @@ jobs:
       with:
         distribution: 'temurin'
         java-version: ${{ matrix.java_version }}
+    - name: Install Maven
+      run: |
+        curl -o ./apache-maven-3.9.9-bin.tar.gz https://dlcdn.apache.org/maven/maven-3/3.9.9/binaries/apache-maven-3.9.9-bin.tar.gz
+        tar -xf ./apache-maven-3.9.9-bin.tar.gz
     - name: Maven Build
       run: |
-        mvn --show-version \
+        ./apache-maven-3.9.9/bin/mvn --show-version \
             --no-transfer-progress \
             --fail-at-end \
             --activate-profiles staging \

modules/http/src/main/java/org/glassfish/grizzly/http/ChunkedTransferEncoding.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2020 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2024 Oracle and/or its affiliates. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0, which is available at
@@ -49,6 +49,9 @@ public final class ChunkedTransferEncoding implements TransferEncoding {
     private static final byte[] LAST_CHUNK_CRLF_BYTES = "0\r\n".getBytes(ASCII_CHARSET);
     private static final int[] DEC = HexUtils.getDecBytes();
 
+    public static final String STRICT_CHUNKED_TRANSFER_CODING_LINE_TERMINATOR_RFC_9112 = "org.glassfish.grizzly.http.STRICT_CHUNKED_TRANSFER_CODING_LINE_TERMINATOR_RFC_9112";
+    private static final boolean isStrictChunkedTransferCodingLineTerminatorSet = Boolean.parseBoolean(System.getProperty(STRICT_CHUNKED_TRANSFER_CODING_LINE_TERMINATOR_RFC_9112));
+
     private final int maxHeadersSize;
 
     public ChunkedTransferEncoding(final int maxHeadersSize) {
@@ -247,6 +250,12 @@ private static boolean parseHttpChunkLength(final HttpPacketParsing httpPacket,
                             b == Constants.CR || b == Constants.SEMI_COLON) {
                         parsingState.checkpoint = offset;
                     } else if (b == Constants.LF) {
+                        if (isStrictChunkedTransferCodingLineTerminatorSet) {
+                            if (parsingState.checkpoint2 == -1 || // no CR
+                                parsingState.checkpoint2 != parsingState.checkpoint) { // not the previous CR or a repetition of a CR
+                                throw new HttpBrokenContentException("Unexpected HTTP chunk header");
+                            }
+                        }
                         final ContentParsingState contentParsingState = httpPacket.getContentParsingState();
                         contentParsingState.chunkContentStart = offset + 1;
                         contentParsingState.chunkLength = value;
@@ -264,6 +273,11 @@ private static boolean parseHttpChunkLength(final HttpPacketParsing httpPacket,
                     } else {
                         throw new HttpBrokenContentException("Unexpected HTTP chunk header");
                     }
+                    if (isStrictChunkedTransferCodingLineTerminatorSet) {
+                        if (b == Constants.CR && parsingState.checkpoint2 == -1) { // first CR
+                            parsingState.checkpoint2 = offset;
+                        }
+                    }
 
                     offset++;
                 }

modules/http/src/test/java/org/glassfish/grizzly/http/ChunkedTransferEncodingTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2020 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2024 Oracle and/or its affiliates. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0, which is available at
@@ -20,6 +20,7 @@
 import static java.lang.Boolean.TRUE;
 import static java.util.Arrays.asList;
 import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.glassfish.grizzly.http.ChunkedTransferEncoding.STRICT_CHUNKED_TRANSFER_CODING_LINE_TERMINATOR_RFC_9112;
 import static org.glassfish.grizzly.http.HttpCodecFilter.DEFAULT_MAX_HTTP_PACKET_HEADER_SIZE;
 import static org.glassfish.grizzly.http.util.MimeHeaders.MAX_NUM_HEADERS_UNBOUNDED;
 import static org.glassfish.grizzly.memory.Buffers.EMPTY_BUFFER;
@@ -90,6 +91,7 @@ static int PORT() {
 
     private final String eol;
     private final boolean isChunkWhenParsing;
+    private final boolean isStrictChunkedTransferCodingLineTerminatorSet;
 
     private TCPNIOTransport transport;
     private Connection connection;
@@ -153,6 +155,8 @@ public void after() throws Exception {
     public ChunkedTransferEncodingTest(String eol, boolean isChunkWhenParsing) {
         this.eol = eol;
         this.isChunkWhenParsing = isChunkWhenParsing;
+        this.isStrictChunkedTransferCodingLineTerminatorSet =
+                Boolean.parseBoolean(System.getProperty(STRICT_CHUNKED_TRANSFER_CODING_LINE_TERMINATOR_RFC_9112));
     }
 
     @Test
@@ -258,8 +262,8 @@ public void testSpacesInChunkSizeHeader() throws Exception {
         sb.append("POST / HTTP/1.1\r\n");
         sb.append("Host: localhost:").append(PORT).append("\r\n");
         sb.append("Transfer-Encoding: chunked\r\n\r\n");
-        sb.append("  ").append(msgLen).append("  ").append(eol).append(msg).append(eol);
-        sb.append("  0  ").append(eol).append(eol);
+        sb.append("  ").append(msgLen).append("  ").append("\r\n").append(msg).append(eol);
+        sb.append("  0  ").append("\r\n").append(eol);
 
         Buffer b = Buffers.wrap(DEFAULT_MEMORY_MANAGER, sb.toString(), Charsets.ASCII_CHARSET);
         Future f = connection.write(b);
@@ -268,6 +272,65 @@ public void testSpacesInChunkSizeHeader() throws Exception {
         assertTrue(result.get(10, SECONDS));
     }
 
+    @SuppressWarnings("unchecked")
+    @Test
+    public void testVulnerableLineTerminatorInChunkSizeHeader() throws Exception {
+        StringBuilder sb = new StringBuilder();
+        String nestedMsg = "XX";
+        String nestedMsgLen = Integer.toHexString(nestedMsg.length());
+        sb.append("\r\n");
+        sb.append("POST /2 HTTP/1.1").append("\r\n");
+        sb.append("Host: localhost:").append(PORT).append("\r\n");
+        sb.append("Transfer-Encoding: chunked").append("\r\n");
+        sb.append("\r\n");
+        sb.append(nestedMsgLen).append("\r\n");
+        sb.append(nestedMsg).append("\r\n");
+        String dummy = sb.toString();
+        String firstMsg = "A".repeat(dummy.length());
+        final String firstMsgLen = Integer.toHexString(firstMsg.length());
+
+        // original packet
+        sb = new StringBuilder();
+        sb.append("POST /1 HTTP/1.1").append("\r\n");
+        sb.append("Host: localhost:").append(PORT).append("\r\n");
+        sb.append("Transfer-Encoding: chunked").append("\r\n");
+        sb.append("\r\n");
+        sb.append(firstMsgLen).append(';').append('\n').append(firstMsg).append('\n').append('0').append("\r\n");
+        sb.append(dummy);
+        sb.append("0").append("\r\n"); // last-chunk
+        sb.append("\r\n"); // CRLF
+
+        final Buffer expectedContent = Buffers.wrap(DEFAULT_MEMORY_MANAGER, firstMsg, ASCII_CHARSET);
+        httpRequestCheckFilter.setCheckParameters(expectedContent, Collections.<String, Pair<String, String>>emptyMap());
+        Buffer b = Buffers.wrap(DEFAULT_MEMORY_MANAGER, sb.toString(), Charsets.ASCII_CHARSET);
+        Future f = connection.write(b);
+        f.get(5, SECONDS);
+
+        Future<Boolean> result;
+        if (!isStrictChunkedTransferCodingLineTerminatorSet) {
+            // first msg
+            result = resultQueue.poll(5, SECONDS);
+            assertTrue(result.get(2, SECONDS));
+
+            // nested msg
+            result = resultQueue.poll(5, SECONDS);
+            try {
+                result.get(2, SECONDS);
+                fail("Expected AssertError to be thrown on server side");
+            } catch (ExecutionException ignore) {
+            }
+        } else {
+            // first msg
+            result = resultQueue.poll(5, SECONDS);
+            try {
+                result.get(2, SECONDS);
+                fail("Expected HttpBrokenContentException to be thrown on server side");
+            } catch (ExecutionException ee) {
+                assertEquals(HttpBrokenContentException.class, ee.getCause().getClass());
+            }
+        }
+    }
+
     /**
      * Test private method {@link ChunkedTransferEncoding#checkOverflow(long)} via reflection.
      *
@@ -319,10 +382,10 @@ private void doHttpRequestTest(int packetsNum, boolean hasContent, Map<String, P
             sb.append(eol);
 
             if (hasContent) {
-                sb.append("3").append(eol).append("a=0").append(eol).append("4").append(eol).append("&b=1").append(eol);
+                sb.append("3").append("\r\n").append("a=0").append(eol).append("4").append("\r\n").append("&b=1").append(eol);
             }
 
-            sb.append("0").append(eol);
+            sb.append("0").append("\r\n");
 
             for (Entry<String, Pair<String, String>> entry : trailerHeaders.entrySet()) {
                 final String value = entry.getValue().getFirst();