Permalink
Browse files

Limit the size of parameter keys

Signed-off-by: James Tucker <jftucker@gmail.com>
  • Loading branch information...
1 parent 7b5af22 commit 5b9d09a81a9fdc9475f0ab0095cb2a33bf2a8f91 @evanphx evanphx committed with raggi Dec 13, 2011
Showing with 77 additions and 0 deletions.
  1. +10 −0 lib/rack/multipart/parser.rb
  2. +30 −0 lib/rack/utils.rb
  3. +11 −0 test/spec_multipart.rb
  4. +26 −0 test/spec_request.rb
View
10 lib/rack/multipart/parser.rb
@@ -14,6 +14,9 @@ def parse
fast_forward_to_first_boundary
+ max_key_space = Utils.key_space_limit
+ bytes = 0
+
loop do
head, filename, content_type, name, body =
get_current_head_and_filename_and_content_type_and_name_and_body
@@ -28,6 +31,13 @@ def parse
filename, data = get_data(filename, body, content_type, name, head)
+ if name
+ bytes += name.size
+ if bytes > max_key_space
+ raise RangeError, "exceeded available parameter key space"
+ end
+ end
+
Utils.normalize_params(@params, name, data) unless data.nil?
# break if we're at the end of a buffer, but not if it is the end of a field
View
30 lib/rack/utils.rb
@@ -47,6 +47,14 @@ def unescape(s, encoding = nil)
DEFAULT_SEP = /[&;] */n
+ class << self
+ attr_accessor :key_space_limit
+ end
+
+ # The default number of bytes to allow parameter keys to take up.
+ # This helps prevent a rogue client from flooding a Request.
+ self.key_space_limit = 65536
+
# Stolen from Mongrel, with some small modifications:
# Parses a query string by breaking it up at the '&'
# and ';' characters. You can also use this to parse
@@ -55,8 +63,19 @@ def unescape(s, encoding = nil)
def parse_query(qs, d = nil)
params = {}
+ max_key_space = Utils.key_space_limit
+ bytes = 0
+
(qs || '').split(d ? /[#{d}] */n : DEFAULT_SEP).each do |p|
k, v = p.split('=', 2).map { |x| unescape(x) }
+
+ if k
+ bytes += k.size
+ if bytes > max_key_space
+ raise RangeError, "exceeded available parameter key space"
+ end
+ end
+
if cur = params[k]
if cur.class == Array
params[k] << v
@@ -75,8 +94,19 @@ def parse_query(qs, d = nil)
def parse_nested_query(qs, d = nil)
params = {}
+ max_key_space = Utils.key_space_limit
+ bytes = 0
+
(qs || '').split(d ? /[#{d}] */n : DEFAULT_SEP).each do |p|
k, v = p.split('=', 2).map { |s| unescape(s) }
+
+ if k
+ bytes += k.size
+ if bytes > max_key_space
+ raise RangeError, "exceeded available parameter key space"
+ end
+ end
+
normalize_params(params, k, v)
end
View
11 test/spec_multipart.rb
@@ -30,6 +30,17 @@ def multipart_file(name)
params["text"].should.equal "contents"
end
+ should "raise RangeError if the key space is exhausted" do
+ env = Rack::MockRequest.env_for("/", multipart_fixture(:content_type_and_no_filename))
+
+ old, Rack::Utils.key_space_limit = Rack::Utils.key_space_limit, 1
+ begin
+ lambda { Rack::Multipart.parse_multipart(env) }.should.raise(RangeError)
+ ensure
+ Rack::Utils.key_space_limit = old
+ end
+ end
+
should "parse multipart form webkit style" do
env = Rack::MockRequest.env_for '/', multipart_fixture(:webkit)
env['CONTENT_TYPE'] = "multipart/form-data; boundary=----WebKitFormBoundaryWLHCs9qmcJJoyjKR"
View
26 test/spec_request.rb
@@ -125,6 +125,18 @@
req.params.should.equal "foo" => "bar", "quux" => "bla"
end
+ should "limit the keys from the GET query string" do
+ env = Rack::MockRequest.env_for("/?foo=bar")
+
+ old, Rack::Utils.key_space_limit = Rack::Utils.key_space_limit, 1
+ begin
+ req = Rack::Request.new(env)
+ lambda { req.GET }.should.raise(RangeError)
+ ensure
+ Rack::Utils.key_space_limit = old
+ end
+ end
+
should "not unify GET and POST when calling params" do
mr = Rack::MockRequest.env_for("/?foo=quux",
"REQUEST_METHOD" => 'POST',
@@ -157,6 +169,20 @@
req.params.should.equal "foo" => "bar", "quux" => "bla"
end
+ should "limit the keys from the POST form data" do
+ env = Rack::MockRequest.env_for("",
+ "REQUEST_METHOD" => 'POST',
+ :input => "foo=bar&quux=bla")
+
+ old, Rack::Utils.key_space_limit = Rack::Utils.key_space_limit, 1
+ begin
+ req = Rack::Request.new(env)
+ lambda { req.POST }.should.raise(RangeError)
+ ensure
+ Rack::Utils.key_space_limit = old
+ end
+ end
+
should "parse POST data with explicit content type regardless of method" do
req = Rack::Request.new \
Rack::MockRequest.env_for("/",

0 comments on commit 5b9d09a

Please sign in to comment.