Merge pull request from GHSA-68xg-gqqm-vgj8

* Reject empty string for Content-Length

* Ignore trailers in last chunk

* test_puma_server.rb - use heredoc, test_cl_and_te_smuggle

* client.rb - stye/RubyCop

* test_puma_server.rb - indented heredoc rubocop disable

* Dentarg comments

* Remove unused variable

---------

Co-authored-by: MSP-Greg <Greg.mpls@gmail.com>

lib/puma/client.rb

@@ -49,7 +49,8 @@ class Client # :nodoc:
 
     # chunked body validation
     CHUNK_SIZE_INVALID = /[^\h]/.freeze
-    CHUNK_VALID_ENDING = "\r\n".freeze
+    CHUNK_VALID_ENDING = Const::LINE_END
+    CHUNK_VALID_ENDING_SIZE = CHUNK_VALID_ENDING.bytesize
 
     # Content-Length header value validation
     CONTENT_LENGTH_VALUE_INVALID = /[^\d]/.freeze
@@ -382,8 +383,8 @@ def setup_body
       cl = @env[CONTENT_LENGTH]
 
       if cl
-        # cannot contain characters that are not \d
-        if CONTENT_LENGTH_VALUE_INVALID.match? cl
+        # cannot contain characters that are not \d, or be empty
+        if CONTENT_LENGTH_VALUE_INVALID.match?(cl) || cl.empty?
           raise HttpParserError, "Invalid Content-Length: #{cl.inspect}"
         end
       else
@@ -544,7 +545,7 @@ def decode_chunk(chunk)
 
       while !io.eof?
         line = io.gets
-        if line.end_with?("\r\n")
+        if line.end_with?(CHUNK_VALID_ENDING)
           # Puma doesn't process chunk extensions, but should parse if they're
           # present, which is the reason for the semicolon regex
           chunk_hex = line.strip[/\A[^;]+/]
@@ -556,13 +557,19 @@ def decode_chunk(chunk)
             @in_last_chunk = true
             @body.rewind
             rest = io.read
-            last_crlf_size = "\r\n".bytesize
-            if rest.bytesize < last_crlf_size
+            if rest.bytesize < CHUNK_VALID_ENDING_SIZE
               @buffer = nil
-              @partial_part_left = last_crlf_size - rest.bytesize
+              @partial_part_left = CHUNK_VALID_ENDING_SIZE - rest.bytesize
               return false
             else
-              @buffer = rest[last_crlf_size..-1]
+              # if the next character is a CRLF, set buffer to everything after that CRLF
+              start_of_rest = if rest.start_with?(CHUNK_VALID_ENDING)
+                CHUNK_VALID_ENDING_SIZE
+              else # we have started a trailer section, which we do not support. skip it!
+                rest.index(CHUNK_VALID_ENDING*2) + CHUNK_VALID_ENDING_SIZE*2
+              end
+
+              @buffer = rest[start_of_rest..-1]
               @buffer = nil if @buffer.empty?
               set_ready
               return true

test/test_puma_server.rb

@@ -749,7 +749,7 @@ def test_large_chunked_request
       [200, {}, [""]]
     }
 
-    header = "GET / HTTP/1.1\r\nConnection: close\r\nTransfer-Encoding: chunked\r\n\r\n"
+    header = "GET / HTTP/1.1\r\nConnection: close\r\nContent-Length: 200\r\nTransfer-Encoding: chunked\r\n\r\n"
 
     chunk_header_size = 6 # 4fb8\r\n
     # Current implementation reads one chunk of CHUNK_SIZE, then more chunks of size 4096.
@@ -1710,4 +1710,82 @@ def test_lowlevel_error_handler_response
 
     assert_match(/something wrong happened/, data)
   end
+
+  def test_cl_empty_string
+    server_run do |env|
+      [200, {}, [""]]
+    end
+
+    # rubocop:disable Layout/TrailingWhitespace
+    empty_cl_request = <<~REQ.gsub("\n", "\r\n")
+      GET / HTTP/1.1
+      Host: localhost
+      Content-Length: 
+      
+      GET / HTTP/1.1
+      Host: localhost
+      
+    REQ
+    # rubocop:enable Layout/TrailingWhitespace
+
+    data = send_http_and_read empty_cl_request
+    assert_operator data, :start_with?, 'HTTP/1.1 400 Bad Request'
+  end
+
+  def test_crlf_trailer_smuggle
+    server_run do |env|
+      [200, {}, [""]]
+    end
+
+    smuggled_payload = <<~REQ.gsub("\n", "\r\n")
+      GET / HTTP/1.1
+      Transfer-Encoding: chunked
+      Host: whatever
+
+      0
+      X:POST / HTTP/1.1
+      Host: whatever
+
+      GET / HTTP/1.1
+      Host: whatever
+
+    REQ
+
+    data = send_http_and_read smuggled_payload
+    assert_equal 2, data.scan("HTTP/1.1 200 OK").size
+  end
+
+  # test to check if content-length is ignored when 'transfer-encoding: chunked'
+  # is used.  See also test_large_chunked_request
+  def test_cl_and_te_smuggle
+    body = nil
+    server_run { |env|
+      body = env['rack.input'].read
+      [200, {}, [""]]
+    }
+
+    req = <<~REQ.gsub("\n", "\r\n")
+      POST /search HTTP/1.1
+      Host: vulnerable-website.com
+      Content-Type: application/x-www-form-urlencoded
+      Content-Length: 4
+      Transfer-Encoding: chunked
+
+      7b
+      GET /404 HTTP/1.1
+      Host: vulnerable-website.com
+      Content-Type: application/x-www-form-urlencoded
+      Content-Length: 144
+
+      x=
+      0
+
+    REQ
+
+    data = send_http_and_read req
+
+    assert_includes body, "GET /404 HTTP/1.1\r\n"
+    assert_includes body, "Content-Length: 144\r\n"
+    assert_equal 1, data.scan("HTTP/1.1 200 OK").size
+  end
 end