Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Deep Munge the parameters for GET and POST

The previous implementation of this functionality could be accidentally
subverted by instantiating a raw Rack::Request before the first Rails::Request
was constructed.

Fixes CVE-2013-6417
  • Loading branch information...
commit ed065b2f693e1f9ef6aa6347f53e5258b1acb1b8 1 parent 4b4f584
@NZKoz NZKoz authored tenderlove committed
View
4 actionpack/lib/action_dispatch/http/request.rb
@@ -271,7 +271,7 @@ def session_options=(options)
# Override Rack's GET method to support indifferent access
def GET
- @env["action_dispatch.request.query_parameters"] ||= (normalize_encode_params(super) || {})
+ @env["action_dispatch.request.query_parameters"] ||= deep_munge((normalize_encode_params(super) || {}))
rescue TypeError => e
raise ActionController::BadRequest.new(:query, e)
end
@@ -279,7 +279,7 @@ def GET
# Override Rack's POST method to support indifferent access
def POST
- @env["action_dispatch.request.request_parameters"] ||= (normalize_encode_params(super) || {})
+ @env["action_dispatch.request.request_parameters"] ||= deep_munge((normalize_encode_params(super) || {}))
rescue TypeError => e
raise ActionController::BadRequest.new(:request, e)
end
View
15 actionpack/test/dispatch/request/query_string_parsing_test.rb
@@ -11,6 +11,17 @@ def parse
head :ok
end
end
+ class EarlyParse
+ def initialize(app)
+ @app = app
+ end
+
+ def call(env)
+ # Trigger a Rack parse so that env caches the query params
+ Rack::Request.new(env).params
+ @app.call(env)
+ end
+ end
def teardown
TestController.last_query_parameters = nil
@@ -131,6 +142,10 @@ def assert_parses(expected, actual)
set.draw do
get ':action', :to => ::QueryStringParsingTest::TestController
end
+ @app = self.class.build_app(set) do |middleware|
+ middleware.use(EarlyParse)
+ end
+
get "/parse", actual
assert_response :ok
Please sign in to comment.
Something went wrong with that request. Please try again.