Skip to content

Commit

Permalink
Limit max size and number of parameters parsed for Content-Disposition
Browse files Browse the repository at this point in the history
Not strictly necessary, but this limits the damage in pathological
cases.  These limits are probably already too generous, we could
probably get by with 8 params and 1024 bytes.  One of tests uses
more than 1024 bytes, though.  Still, it seems unlikely any
legitimate requests would exceed these limits.  We could make the
limits configurable via an accessor method, if desired.
  • Loading branch information
jeremyevans committed Apr 29, 2023
1 parent a54b615 commit ec43511
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 1 deletion.
11 changes: 10 additions & 1 deletion lib/rack/multipart/parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -293,18 +293,27 @@ def handle_consume_token
end
end

CONTENT_DISPOSITION_MAX_PARAMS = 16
CONTENT_DISPOSITION_MAX_BYTES = 1536
def handle_mime_head
if @sbuf.scan_until(@head_regex)
head = @sbuf[1]
content_type = head[MULTIPART_CONTENT_TYPE, 1]
if disposition = head[MULTIPART_CONTENT_DISPOSITION, 1]
if (disposition = head[MULTIPART_CONTENT_DISPOSITION, 1]) &&
disposition.bytesize <= CONTENT_DISPOSITION_MAX_BYTES

# ignore actual content-disposition value (should always be form-data)
i = disposition.index(';')
disposition.slice!(0, i+1)
param = nil
num_params = 0

# Parse parameter list
while i = disposition.index('=')
# Only parse up to 32 parameters, to avoid potential denial of service
num_params += 1
break if num_params > CONTENT_DISPOSITION_MAX_PARAMS

# Found end of parameter name, ensure forward progress in loop
param = disposition.slice!(0, i+1)

Expand Down
26 changes: 26 additions & 0 deletions test/spec_multipart.rb
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,32 @@ def rd.rewind; end
x["file\\-xfoo"][:name].must_equal "file\\-xfoo"
end

it "parse up to 16 content-disposition params" do
x = content_disposition_parse.call("#{14.times.map{|x| "a#{x}=b;"}.join} filename=\"bar\"; name=\"file\"")
x.keys.must_equal ["file"]
x["file"][:filename].must_equal "bar"
x["file"][:name].must_equal "file"
end

it "stop parsing content-disposition after 16 params" do
x = content_disposition_parse.call("#{15.times.map{|x| "a#{x}=b;"}.join} filename=\"bar\"; name=\"file\"")
x.keys.must_equal ["bar"]
x["bar"][:filename].must_equal "bar"
x["bar"][:name].must_equal "bar"
end

it "allow content-disposition values up to 1536 bytes" do
x = content_disposition_parse.call("a=#{'a'*1480}; filename=\"bar\"; name=\"file\"")
x.keys.must_equal ["file"]
x["file"][:filename].must_equal "bar"
x["file"][:name].must_equal "file"
end

it "ignore content-disposition values over to 1536 bytes" do
x = content_disposition_parse.call("a=#{'a'*1510}; filename=\"bar\"; name=\"file\"")
x.must_equal "text/plain"=>[""]
end

it 'raises an EOF error on content-length mismatch' do
env = Rack::MockRequest.env_for("/", multipart_fixture(:empty))
env['rack.input'] = StringIO.new
Expand Down

0 comments on commit ec43511

Please sign in to comment.