Do not allow extra CRs in chunks (#11936)

* Do not allow extra CRs in chunks * Renumber test uuid * Add test cases and fix an oversight * Use prefix increment
apache · Jan 3, 2025 · f5f2256 · f5f2256
1 parent 43b68ee
commit f5f2256
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 6 deletions.
diff --git a/src/proxy/http/HttpTunnel.cc b/src/proxy/http/HttpTunnel.cc
@@ -148,6 +148,7 @@ ChunkedHandler::read_size()
 {
   int64_t bytes_used;
   bool    done = false;
+  int     cr   = 0;
 
   while (chunked_reader->read_avail() > 0 && !done) {
     const char *tmp       = chunked_reader->start();
@@ -186,6 +187,9 @@ ChunkedHandler::read_size()
             done  = true;
             break;
           } else {
+            if (ParseRules::is_cr(*tmp)) {
+              ++cr;
+            }
             state = CHUNK_READ_SIZE_CRLF; // now look for CRLF
           }
         }
@@ -195,7 +199,15 @@ ChunkedHandler::read_size()
           cur_chunk_bytes_left = (cur_chunk_size = running_sum);
           state                = (running_sum == 0) ? CHUNK_READ_TRAILER_BLANK : CHUNK_READ_CHUNK;
           done                 = true;
+          cr                   = 0;
           break;
+        } else if (ParseRules::is_cr(*tmp)) {
+          if (cr != 0) {
+            state = CHUNK_READ_ERROR;
+            done  = true;
+            break;
+          }
+          ++cr;
         }
       } else if (state == CHUNK_READ_SIZE_START) {
         if (ParseRules::is_cr(*tmp)) {

diff --git a/tests/gold_tests/chunked_encoding/bad_chunked_encoding.test.py b/tests/gold_tests/chunked_encoding/bad_chunked_encoding.test.py
@@ -172,13 +172,13 @@ def runChunkedTraffic(self):
         # code from the verifier client.
         tr.Processes.Default.ReturnCode = 1
         tr.Processes.Default.Streams.stdout += Testers.ContainsExpression(
-            r"(Unexpected chunked content for key 4: too small|Failed HTTP/1 transaction with key: 4)",
+            r"(Unexpected chunked content for key 101: too small|Failed HTTP/1 transaction with key: 101)",
             "Verify that ATS closed the forth transaction.")
         tr.Processes.Default.Streams.stdout += Testers.ContainsExpression(
-            r"(Unexpected chunked content for key 5: too small|Failed HTTP/1 transaction with key: 5)",
+            r"(Unexpected chunked content for key 102: too small|Failed HTTP/1 transaction with key: 102)",
             "Verify that ATS closed the fifth transaction.")
         tr.Processes.Default.Streams.stdout += Testers.ContainsExpression(
-            r"(Unexpected chunked content for key 6: too small|Failed HTTP/1 transaction with key: 6)",
+            r"(Unexpected chunked content for key 103: too small|Failed HTTP/1 transaction with key: 103)",
             "Verify that ATS closed the sixth transaction.")
 
         # ATS should close the connection before any body gets through. "def"

diff --git a/tests/gold_tests/chunked_encoding/replays/malformed_chunked_header.replay.yaml b/tests/gold_tests/chunked_encoding/replays/malformed_chunked_header.replay.yaml
@@ -78,6 +78,26 @@ sessions:
     server-response:
       status: 200
 
+- transactions:
+  - client-request:
+      method: "POST"
+      version: "1.1"
+      url: /malformed/chunk/header3
+      headers:
+        fields:
+        - [ Host, example.com ]
+        - [ Transfer-Encoding, chunked ]
+        - [ uuid, 4 ]
+      content:
+        transfer: plain
+        encoding: uri
+        # BWS cannot have CR
+        data: 3%0D%0D%0Aabc%0D%0A0%0D%0A%0D%0A
+
+    # The connection will be dropped and this response will not go out.
+    server-response:
+      status: 200
+
   #
   # Now repeat the above two malformed chunk header tests, but on the server
   # side.
@@ -90,7 +110,7 @@ sessions:
       headers:
         fields:
         - [ Host, example.com ]
-        - [ uuid, 4 ]
+        - [ uuid, 101 ]
 
     # The connection will be dropped and this response will not go out.
     server-response:
@@ -113,7 +133,7 @@ sessions:
       headers:
         fields:
         - [ Host, example.com ]
-        - [ uuid, 5 ]
+        - [ uuid, 102 ]
 
     # The connection will be dropped and this response will not go out.
     server-response:
@@ -136,7 +156,7 @@ sessions:
       headers:
         fields:
         - [ Host, example.com ]
-        - [ uuid, 6 ]
+        - [ uuid, 103 ]
 
     # The connection will be dropped and this response will not go out.
     server-response:
@@ -150,3 +170,26 @@ sessions:
         encoding: uri
         # Super large chunk header, larger than will fit in an int.
         data: 111111113%0D%0Adef%0D%0A0%0D%0A%0D%0A
+
+- transactions:
+  - client-request:
+      method: "GET"
+      version: "1.1"
+      url: /response/malformed/chunk/size2
+      headers:
+        fields:
+        - [ Host, example.com ]
+        - [ uuid, 104 ]
+
+    # The connection will be dropped and this response will not go out.
+    server-response:
+      status: 200
+      reason: OK
+      headers:
+        fields:
+        - [ Transfer-Encoding, chunked ]
+      content:
+        transfer: plain
+        encoding: uri
+        # BWS cannot have CR
+        data: 3%0D%0D%0Aabc%0D%0A0%0D%0A%0D%0A