File rubygem-rack-CVE-2023-27530.patch of Package rubygem-rack.28303

Overview Repositories Revisions Requests Users Attributes Meta

File rubygem-rack-CVE-2023-27530.patch of Package rubygem-rack.28303

From 5f6e2fcbbdbff2dfaa21baa693e9d23d12ac1459 Mon Sep 17 00:00:00 2001
From: John Hawthorn <john@hawthorn.email>
Date: Thu, 8 Dec 2022 15:54:28 -0800
Subject: [PATCH] Limit all multipart parts, not just files

Previously we would limit the number of multipart parts which were
files, but not other parts. In some cases this could cause parsing of
maliciously crafted inputs to take longer than expected.

[CVE-2023-27530]
---
 README.rdoc                  | 20 +++++++++++++++++---
 lib/rack/multipart/parser.rb | 20 ++++++++++++++++----
 lib/rack/utils.rb            | 19 +++++++++++++++----
 test/spec_multipart.rb       | 12 ++++++++++++
 test/spec_request.rb         | 18 +++++++++++++++++-
 5 files changed, 77 insertions(+), 12 deletions(-)

Index: rack-2.0.8/README.rdoc
===================================================================
--- rack-2.0.8.orig/README.rdoc
+++ rack-2.0.8/README.rdoc
@@ -189,16 +189,30 @@ This helps prevent a rogue client from f
 
 Default to 65536 characters (4 kiB in worst case).
 
-=== multipart_part_limit
+=== multipart_file_limit
 
-The maximum number of parts a request can contain.
+The maximum number of parts with a filename a request can contain.
 Accepting too many part can lead to the server running out of file handles.
 
 The default is 128, which means that a single request can't upload more than 128 files at once.
 
 Set to 0 for no limit.
 
-Can also be set via the RACK_MULTIPART_PART_LIMIT environment variable.
+Can also be set via the +RACK_MULTIPART_FILE_LIMIT+ environment variable.
+
+(This is also aliased as +multipart_part_limit+ and +RACK_MULTIPART_PART_LIMIT+ for compatibility)
+
+=== multipart_total_part_limit
+
+The maximum total number of parts a request can contain of any type, including
+both file and non-file form fields.
+
+The default is 4096, which means that a single request can't contain more than
+4096 parts.
+
+Set to 0 for no limit.
+
+Can also be set via the +RACK_MULTIPART_TOTAL_PART_LIMIT+ environment variable.
 
 == History
 
Index: rack-2.0.8/lib/rack/multipart/parser.rb
===================================================================
--- rack-2.0.8.orig/lib/rack/multipart/parser.rb
+++ rack-2.0.8/lib/rack/multipart/parser.rb
@@ -3,6 +3,7 @@ require 'rack/utils'
 module Rack
   module Multipart
     class MultipartPartLimitError < Errno::EMFILE; end
+    class MultipartTotalPartLimitError < StandardError; end
 
     class Parser
       BUFSIZE = 16384
@@ -138,7 +139,8 @@ module Rack
           end
 
           @mime_parts[mime_index] = klass.new(body, head, filename, content_type, name)
-          check_open_files
+
+          check_part_limits
         end
 
         def on_mime_body mime_index, content
@@ -150,13 +152,23 @@ module Rack
 
         private
 
-        def check_open_files
-          if Utils.multipart_part_limit > 0
-            if @open_files >= Utils.multipart_part_limit
+        def check_part_limits
+          file_limit = Utils.multipart_file_limit
+          part_limit = Utils.multipart_total_part_limit
+
+          if file_limit && file_limit > 0
+            if @open_files >= file_limit
               @mime_parts.each(&:close)
               raise MultipartPartLimitError, 'Maximum file multiparts in content reached'
             end
           end
+
+          if part_limit && part_limit > 0
+            if @mime_parts.size >= part_limit
+              @mime_parts.each(&:close)
+              raise MultipartTotalPartLimitError, 'Maximum total multiparts in content reached'
+            end
+          end
         end
       end
 
Index: rack-2.0.8/lib/rack/utils.rb
===================================================================
--- rack-2.0.8.orig/lib/rack/utils.rb
+++ rack-2.0.8/lib/rack/utils.rb
@@ -53,13 +53,24 @@ module Rack
     module_function :unescape
 
     class << self
-      attr_accessor :multipart_part_limit
+      attr_accessor :multipart_total_part_limit
+
+      attr_accessor :multipart_file_limit
+
+      # multipart_part_limit is the original name of multipart_file_limit, but
+      # the limit only counts parts with filenames.
+      alias multipart_part_limit multipart_file_limit
+      alias multipart_part_limit= multipart_file_limit=
     end
 
-    # The maximum number of parts a request can contain. Accepting too many part
-    # can lead to the server running out of file handles.
+    # The maximum number of file parts a request can contain. Accepting too
+    # many parts can lead to the server running out of file handles.
     # Set to `0` for no limit.
-    self.multipart_part_limit = (ENV['RACK_MULTIPART_PART_LIMIT'] || 128).to_i
+    self.multipart_file_limit = (ENV['RACK_MULTIPART_PART_LIMIT'] || ENV['RACK_MULTIPART_FILE_LIMIT'] || 128).to_i
+
+    # The maximum total number of parts a request can contain. Accepting too
+    # many can lead to excessive memory use and parsing time.
+    self.multipart_total_part_limit = (ENV['RACK_MULTIPART_TOTAL_PART_LIMIT'] || 4096).to_i
 
     def self.param_depth_limit
       default_query_parser.param_depth_limit
Index: rack-2.0.8/test/spec_multipart.rb
===================================================================
--- rack-2.0.8.orig/test/spec_multipart.rb
+++ rack-2.0.8/test/spec_multipart.rb
@@ -561,6 +561,18 @@ Content-Type: image/jpeg\r
     end
   end
 
+  it "reach a multipart total limit" do
+    begin
+      previous_limit = Rack::Utils.multipart_total_part_limit
+      Rack::Utils.multipart_total_part_limit = 5
+
+      env = Rack::MockRequest.env_for '/', multipart_fixture(:three_files_three_fields)
+      lambda { Rack::Multipart.parse_multipart(env) }.must_raise Rack::Multipart::MultipartTotalPartLimitError
+    ensure
+      Rack::Utils.multipart_total_part_limit = previous_limit
+    end
+  end
+
   it "return nil if no UploadedFiles were used" do
     data = Rack::Multipart.build_multipart("people" => [{"submit-name" => "Larry", "files" => "contents"}])
     data.must_be_nil
Index: rack-2.0.8/test/spec_request.rb
===================================================================
--- rack-2.0.8.orig/test/spec_request.rb
+++ rack-2.0.8/test/spec_request.rb
@@ -911,7 +911,7 @@ EOF
     f[:tempfile].size.must_equal 76
   end
 
-  it "MultipartPartLimitError when request has too many multipart parts if limit set" do
+  it "MultipartPartLimitError when request has too many multipart file parts if limit set" do
     begin
       data = 10000.times.map { "--AaB03x\r\nContent-Type: text/plain\r\nContent-Disposition: attachment; name=#{SecureRandom.hex(10)}; filename=#{SecureRandom.hex(10)}\r\n\r\ncontents\r\n" }.join("\r\n")
       data += "--AaB03x--\r"
@@ -927,6 +927,22 @@ EOF
     end
   end
 
+  it "MultipartPartLimitError when request has too many multipart total parts if limit set" do
+    begin
+      data = 10000.times.map { "--AaB03x\r\ncontent-type: text/plain\r\ncontent-disposition: attachment; name=#{SecureRandom.hex(10)}\r\n\r\ncontents\r\n" }.join("\r\n")
+      data += "--AaB03x--\r"
+
+      options = {
+        "CONTENT_TYPE" => "multipart/form-data; boundary=AaB03x",
+        "CONTENT_LENGTH" => data.length.to_s,
+        :input => StringIO.new(data)
+      }
+
+      request = make_request Rack::MockRequest.env_for("/", options)
+      lambda { request.POST }.must_raise Rack::Multipart::MultipartTotalPartLimitError
+    end
+  end
+
   it 'closes tempfiles it created in the case of too many created' do
     begin
       data = 10000.times.map { "--AaB03x\r\nContent-Type: text/plain\r\nContent-Disposition: attachment; name=#{SecureRandom.hex(10)}; filename=#{SecureRandom.hex(10)}\r\n\r\ncontents\r\n" }.join("\r\n")

Places

File rubygem-rack-CVE-2023-27530.patch of Package rubygem-rack.28303

Places