Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix nil array sql vulnerability (CVE-2012-2660 + CVE-2012-2694) #1

Closed
wants to merge 3 commits into from

1 participant

@presidentbeef

When normalizing parameters, also check for [nil].

Intended to fix CVE-2012-2660 and CVE-2012-2694

@presidentbeef

Also added potential fix for CVE-2012-2694, where the query parameter array values might have nil and other values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
6 actionpack/lib/action_controller/request.rb
@@ -486,7 +486,11 @@ def normalize_parameters(value)
h.with_indifferent_access
end
when Array
- value.map { |e| normalize_parameters(e) }
+ if value.length == 1 and value[0] == nil
+ value = nil
+ else
+ value.map { |e| normalize_parameters(e) }.compact
+ end
else
value
end
View
15 actionpack/test/controller/request/query_string_parsing_test.rb
@@ -105,6 +105,21 @@ def teardown
)
end
+ #These tests are copied directly from the official patch
+ test "nested query with nil arrays" do
+ assert_parses({"action" => nil}, "action")
+ assert_parses({"action" => {"foo" => nil}}, "action[foo]")
+ assert_parses({"action" => {"foo" => { "bar" => nil }}}, "action[foo][bar]")
+ assert_parses({"action" => {"foo" => { "bar" => nil }}}, "action[foo][bar][]")
+ assert_parses({"action" => {"foo" => nil}}, "action[foo][]")
+ assert_parses({"action"=>{"foo"=>[{"bar"=>nil}]}}, "action[foo][][bar]")
+ end
+
+ test "remove nils from query parameter arrays" do
+ assert_parses({"action" => ['1']}, "action[]=1&action[]")
+ assert_parses({"action" => ['1', '2']}, "action[]=1&action[]&action[]=2")
+ end
+
private
def assert_parses(expected, actual)
with_routing do |set|
Something went wrong with that request. Please try again.