Skip to content

Commit

Permalink
Limit all multipart parts, not just files
Browse files Browse the repository at this point in the history
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]
  • Loading branch information
jhawthorn authored and tenderlove committed Mar 2, 2023
1 parent 2606ac5 commit 9aac375
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 12 deletions.
20 changes: 17 additions & 3 deletions README.rdoc
Original file line number Diff line number Diff line change
Expand Up @@ -202,16 +202,30 @@ Limiting the depth prevents a possible stack overflow when parsing parameters.

Defaults to 100.

=== 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.

== Changelog

Expand Down
19 changes: 15 additions & 4 deletions lib/rack/multipart/parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
module Rack
module Multipart
class MultipartPartLimitError < Errno::EMFILE; end
class MultipartTotalPartLimitError < StandardError; end

class Parser
(require_relative '../core_ext/regexp'; using ::Rack::RegexpExtensions) if RUBY_VERSION < '2.4'
Expand Down Expand Up @@ -140,7 +141,7 @@ def on_mime_head(mime_index, head, filename, content_type, name)

@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)
Expand All @@ -152,13 +153,23 @@ def on_mime_finish(mime_index)

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

Expand Down
19 changes: 15 additions & 4 deletions lib/rack/utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,24 @@ def unescape(s, encoding = Encoding::UTF_8)
end

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
Expand Down
12 changes: 12 additions & 0 deletions test/spec_multipart.rb
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,18 @@ def initialize(*)
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
Expand Down
18 changes: 17 additions & 1 deletion test/spec_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1000,7 +1000,7 @@ def initialize(*)
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"
Expand All @@ -1016,6 +1016,22 @@ def initialize(*)
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")
Expand Down

0 comments on commit 9aac375

Please sign in to comment.