Unbounded read in Rack::Request form parsing can lead to memory exhaustion.

ioquatix · ioquatix · commit e179614c4a65 · 2025-10-10T13:34:22.000+13:00
- Limit read to `query_parser.bytesize_limit`.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file. For info on
 ### Security
 
 - [CVE-2025-61780](https://github.com/advisories/GHSA-r657-rxjc-j557) Improper handling of headers in `Rack::Sendfile` may allow proxy bypass.
+- [CVE-2025-61919](https://github.com/advisories/GHSA-6xw4-3v39-52mm) Unbounded read in `Rack::Request` form parsing can lead to memory exhaustion.
 
 ## [3.2.2] - 2025-10-07
 
diff --git a/lib/rack/query_parser.rb b/lib/rack/query_parser.rb
@@ -57,6 +57,8 @@ def self.make_default(param_depth_limit, **options)
     PARAMS_LIMIT = env_int.call("RACK_QUERY_PARSER_PARAMS_LIMIT", 4096)
     private_constant :PARAMS_LIMIT
 
+    attr_reader :bytesize_limit
+
     def initialize(params_class, param_depth_limit, bytesize_limit: BYTESIZE_LIMIT, params_limit: PARAMS_LIMIT)
       @params_class = params_class
       @param_depth_limit = param_depth_limit
@@ -221,7 +223,7 @@ def each_query_pair(qs, separator, unescaper = nil)
       return if !qs || qs.empty?
 
       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
 
       pairs = qs.split(separator ? (COMMON_SEP[separator] || /[#{separator}] */n) : DEFAULT_SEP, @params_limit + 1)
diff --git a/lib/rack/request.rb b/lib/rack/request.rb
@@ -513,7 +513,10 @@ def form_pairs
             if pairs = Rack::Multipart.parse_multipart(env, Rack::Multipart::ParamList)
               set_header RACK_REQUEST_FORM_PAIRS, pairs
             else
-              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:
diff --git a/test/spec_request.rb b/test/spec_request.rb
@@ -795,6 +795,84 @@ def initialize(*)
     req.POST.must_equal "foo" => "bar", "quux" => "bla"
   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
+
+    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
+
+    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
+
+    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 "return form_pairs for url-encoded POST data" do
     req = make_request \
       Rack::MockRequest.env_for("/",
