From 4e2c903991a790ee211a3021808ff4fd6fe82881 Mon Sep 17 00:00:00 2001
From: Samuel Williams <samuel.williams@oriontransfer.co.nz>
Date: Thu, 9 Oct 2025 20:38:58 +1300
Subject: [PATCH] Unbounded read in `Rack::Request` form parsing can lead to
 memory exhaustion.

- Limit read to `query_parser.bytesize_limit`.
---
 CHANGELOG.md             |  1 +
 lib/rack/query_parser.rb |  4 +-
 lib/rack/request.rb      |  5 ++-
 test/spec_request.rb     | 87 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 95 insertions(+), 2 deletions(-)

--- a/lib/rack/query_parser.rb
+++ b/lib/rack/query_parser.rb
@@ -53,6 +53,8 @@
     PARAMS_LIMIT = env_int.call("RACK_QUERY_PARSER_PARAMS_LIMIT", 4096)
     private_constant :PARAMS_LIMIT
 
+    attr_reader :bytesize_limit
+
     def initialize(params_class, key_space_limit, param_depth_limit, bytesize_limit: BYTESIZE_LIMIT, params_limit: PARAMS_LIMIT)
       @params_class = params_class
       @key_space_limit = key_space_limit
@@ -186,7 +188,7 @@
     def check_query_string(qs, sep)
       if qs
         if qs.bytesize > @bytesize_limit
-          raise QueryLimitError, "total query size (#{qs.bytesize}) exceeds limit (#{@bytesize_limit})"
+          raise QueryLimitError, "total query size exceeds limit (#{@bytesize_limit})"
         end
 
         if (param_count = qs.count(sep.is_a?(String) ? sep : '&;')) >= @params_limit
--- a/lib/rack/request.rb
+++ b/lib/rack/request.rb
@@ -354,7 +354,10 @@
           get_header(RACK_REQUEST_FORM_HASH)
         elsif form_data? || parseable_data?
           unless set_header(RACK_REQUEST_FORM_HASH, parse_multipart)
-            form_vars = get_header(RACK_INPUT).read
+            # Add 2 bytes. One to check whether it is over the limit, and a second
+            # in case the slice! call below removes the last byte
+            # If read returns nil, use the empty string
+            form_vars = get_header(RACK_INPUT).read(query_parser.bytesize_limit + 2) || ''
 
             # Fix for Safari Ajax postings that always append \0
             # form_vars.sub!(/\0\z/, '') # performance replacement:
--- a/test/spec_request.rb
+++ b/test/spec_request.rb
@@ -445,6 +445,93 @@
     req.params.wont_be_same_as req.GET
   end
 
+  it "limit POST body read to bytesize_limit when parsing url-encoded data" do
+    # Create a mock input that tracks read calls
+    reads = []
+    mock_input = Object.new
+    mock_input.define_singleton_method(:read) do |len=nil|
+      reads << len
+      # Return mutable string
+      "foo=bar".dup
+    end
+    mock_input.define_singleton_method(:rewind) do
+      # no-op for compatibility
+    end
+
+    request = make_request \
+      Rack::MockRequest.env_for("/",
+        'REQUEST_METHOD' => 'POST',
+        'CONTENT_TYPE' => 'application/x-www-form-urlencoded',
+        'rack.input' => mock_input)
+
+    request.POST.must_equal "foo" => "bar"
+
+    # Verify read was called with a limit (bytesize_limit + 2), not nil
+    reads.size.must_equal 1
+    reads.first.wont_be_nil
+    reads.first.must_equal(request.send(:query_parser).bytesize_limit + 2)
+  end
+
+  it "handle nil return from rack.input.read when parsing url-encoded data" do
+    # Simulate an input that returns nil on read
+    mock_input = Object.new
+    mock_input.define_singleton_method(:read) do |len=nil|
+      nil
+    end
+    mock_input.define_singleton_method(:rewind) do
+      # no-op for compatibility
+    end
+
+    request = make_request \
+      Rack::MockRequest.env_for("/",
+        'REQUEST_METHOD' => 'POST',
+        'CONTENT_TYPE' => 'application/x-www-form-urlencoded',
+        'rack.input' => mock_input)
+
+    # Should handle nil gracefully and return empty hash
+    request.POST.must_equal({})
+  end
+
+  it "truncate POST body at bytesize_limit when parsing url-encoded data" do
+    # Create input larger than limit
+    large_body = "a=1&" * 1000000  # Very large body
+
+    request = make_request \
+      Rack::MockRequest.env_for("/",
+        'REQUEST_METHOD' => 'POST',
+        'CONTENT_TYPE' => 'application/x-www-form-urlencoded',
+        :input => large_body)
+
+    # Should parse only up to the limit without reading entire body into memory
+    # The actual parsing may fail due to size limit, which is expected
+    proc { request.POST }.must_raise Rack::QueryParser::QueryLimitError
+  end
+
+  it "clean up Safari's ajax POST body with limited read" do
+    # Verify Safari null-byte cleanup still works with bounded read
+    reads = []
+    mock_input = Object.new
+    mock_input.define_singleton_method(:read) do |len=nil|
+      reads << len
+      # Return mutable string (dup ensures it's not frozen)
+      "foo=bar\0".dup
+    end
+    mock_input.define_singleton_method(:rewind) do
+      # no-op for compatibility
+    end
+
+    request = make_request \
+      Rack::MockRequest.env_for("/",
+        'REQUEST_METHOD' => 'POST',
+        'CONTENT_TYPE' => 'application/x-www-form-urlencoded',
+        'rack.input' => mock_input)
+
+    request.POST.must_equal "foo" => "bar"
+
+    # Verify bounded read was used
+    reads.first.wont_be_nil
+  end
+
   it "get value by key from params with #[]" do
     req = make_request \
       Rack::MockRequest.env_for("?foo=quux")