Skip to content

Commit

Permalink
Deep Munge the parameters for GET and POST
Browse files Browse the repository at this point in the history
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

Conflicts:
	actionpack/lib/action_dispatch/http/request.rb
  • Loading branch information
NZKoz authored and tenderlove committed Dec 2, 2013
1 parent 78790e4 commit d5a4095
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 2 deletions.
4 changes: 2 additions & 2 deletions actionpack/lib/action_dispatch/http/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -228,13 +228,13 @@ def session_options=(options)

# Override Rack's GET method to support indifferent access
def GET
@env["action_dispatch.request.query_parameters"] ||= (normalize_parameters(super) || {})
@env["action_dispatch.request.query_parameters"] ||= deep_munge(normalize_parameters(super) || {})
end
alias :query_parameters :GET

# Override Rack's POST method to support indifferent access
def POST
@env["action_dispatch.request.request_parameters"] ||= (normalize_parameters(super) || {})
@env["action_dispatch.request.request_parameters"] ||= deep_munge(normalize_parameters(super) || {})
end
alias :request_parameters :POST

Expand Down
15 changes: 15 additions & 0 deletions actionpack/test/dispatch/request/query_string_parsing_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -120,6 +131,10 @@ def assert_parses(expected, actual)
set.draw do
match ':action', :to => ::QueryStringParsingTest::TestController
end
@app = self.class.build_app(set) do |middleware|
middleware.use(EarlyParse)
end


get "/parse", actual
assert_response :ok
Expand Down

0 comments on commit d5a4095

Please sign in to comment.