CVE-2023-40175: Inconsistent Interpretation of HTTP Requests ('HTTP Request/Response Smuggling') (CWE-444)
value: Puma is a Ruby/Rack web server built for parallelism. Prior to versions 6.3.1 and 5.6.7, puma exhibited incorrect behavior when parsing chunked transfer encoding bodies and zero-length Content-Length headers in a way that allowed HTTP request smuggling. Severity of this issue is highly dependent on the nature of the web site using puma is. This could be caused by either incorrect parsing of trailing fields in chunked transfer encoding bodies or by parsing of blank/zero-length Content-Length headers. Both issues have been addressed and this vulnerability has been fixed in versions 6.3.1 and 5.6.7. Users are advised to upgrade. There are no known workarounds for this vulnerability.
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: 9.8
Published: 2023-08-18T22:15Z
Repository: https://github.com/puma/puma
Language: Ruby
Files changed: lib/puma/client.rb, test/test_puma_server.rb
Commit date: 2023-08-18 09:47:23 +0900
Commit message: 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>
Diff: diff --git a/lib/puma/client.rb b/lib/puma/client.rb
index 83ec03ae1..6aaab3005 100644
--- a/lib/puma/client.rb
+++ b/lib/puma/client.rb
@@ -47,11 +47,12 @@ module Puma
     # this tests all values but the last, which must be chunked
     ALLOWED_TRANSFER_ENCODING = %w[compress deflate gzip].freeze
 
     # 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
 
     TE_ERR_MSG = 'Invalid Transfer-Encoding'
@@ -380,12 +381,12 @@ module Puma
       @chunked_body = false
 
       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
         @buffer = body.empty? ? nil : body
         @body = EmptyBody
@@ -542,11 +543,11 @@ module Puma
         @prev_chunk = ""
       end
 
       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[^;]+/]
           if CHUNK_SIZE_INVALID.match? chunk_hex
             raise HttpParserError, "Invalid chunk size: '#{chunk_hex}'"
@@ -554,17 +555,23 @@ module Puma
           len = chunk_hex.to_i(16)
           if len == 0
             @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
             end
           end
diff --git a/test/test_puma_server.rb b/test/test_puma_server.rb
index fd6cbc800..0dc7c26e7 100644
--- a/test/test_puma_server.rb
+++ b/test/test_puma_server.rb
@@ -747,11 +747,11 @@ class TestPumaServer < Minitest::Test
       body = env['rack.input'].read
       content_length = env['CONTENT_LENGTH']
       [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.
     # We want a chunk to split exactly after "#{request_body}\r", before the "\n".
     edge_case_size = Puma::Const::CHUNK_SIZE + 4096 - header.size - chunk_header_size - 1
@@ -1708,6 +1708,84 @@ class TestPumaServer < Minitest::Test
 
     data = send_http_and_read "GET / HTTP/1.1\r\n\r\n"
 
     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

Vulnerable code:     CHUNK_VALID_ENDING = "\r\n".freeze
        # cannot contain characters that are not \d
        if CONTENT_LENGTH_VALUE_INVALID.match? cl
        if line.end_with?("\r\n")
            last_crlf_size = "\r\n".bytesize
            if rest.bytesize < last_crlf_size
              @partial_part_left = last_crlf_size - rest.bytesize
              @buffer = rest[last_crlf_size..-1]
    header = "GET / HTTP/1.1\r\nConnection: close\r\nTransfer-Encoding: chunked\r\n\r\n"
Fixed code:     CHUNK_VALID_ENDING = Const::LINE_END
    CHUNK_VALID_ENDING_SIZE = CHUNK_VALID_ENDING.bytesize
        # cannot contain characters that are not \d, or be empty
        if CONTENT_LENGTH_VALUE_INVALID.match?(cl) || cl.empty?
        if line.end_with?(CHUNK_VALID_ENDING)
            if rest.bytesize < CHUNK_VALID_ENDING_SIZE
              @partial_part_left = CHUNK_VALID_ENDING_SIZE - rest.bytesize
              # 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]
    header = "GET / HTTP/1.1\r\nConnection: close\r\nContent-Length: 200\r\nTransfer-Encoding: chunked\r\n\r\n"

  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