Limit max size and number of parameters parsed for Content-Disposition

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.

2 files changed, 36 insertions, 1 deletions

diff --git a/lib/rack/multipart/parser.rb b/lib/rack/multipart/parser.rb
index e0227be3..b514f2bd 100644
--- a/lib/rack/multipart/parser.rb
+++ b/lib/rack/multipart/parser.rb
@@ -293,18 +293,27 @@ module Rack
         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)
 
diff --git a/test/spec_multipart.rb b/test/spec_multipart.rb
index c36fe50c..6acdeeaf 100644
--- a/test/spec_multipart.rb
+++ b/test/spec_multipart.rb
@@ -311,6 +311,32 @@ describe Rack::Multipart do
     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