diff options
| -rw-r--r-- | actionpack/CHANGELOG.md | 11 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/log_subscriber.rb | 9 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/request/utils.rb | 4 | ||||
| -rw-r--r-- | actionpack/test/dispatch/request/json_params_parsing_test.rb | 4 | ||||
| -rw-r--r-- | actionpack/test/dispatch/request/query_string_parsing_test.rb | 4 | ||||
| -rw-r--r-- | guides/source/action_controller_overview.md | 4 | ||||
| -rw-r--r-- | guides/source/security.md | 8 |
7 files changed, 21 insertions, 23 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 3b02994459..115ad54190 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,14 @@ +* Stop converting empty arrays in `params` to `nil` + + This behaviour was introduced in response to CVE-2012-2660, CVE-2012-2694 + and CVE-2013-0155 + + ActiveRecord now issues a safe query when passing an empty array into + a where clause, so there is no longer a need to defend against this type + of input (any nils are still stripped from the array). + + *Chris Sinjakli* + * Fixed usage of optional scopes in URL helpers. *Alex Robbin* diff --git a/actionpack/lib/action_controller/log_subscriber.rb b/actionpack/lib/action_controller/log_subscriber.rb index d3f93a5352..87609d8aa7 100644 --- a/actionpack/lib/action_controller/log_subscriber.rb +++ b/actionpack/lib/action_controller/log_subscriber.rb @@ -53,15 +53,6 @@ module ActionController end end - def deep_munge(event) - debug do - "Value for params[:#{event.payload[:keys].join('][:')}] was set "\ - "to nil, because it was one of [], [null] or [null, null, ...]. "\ - "Go to http://guides.rubyonrails.org/security.html#unsafe-query-generation "\ - "for more information."\ - end - end - %w(write_fragment read_fragment exist_fragment? expire_fragment expire_page write_page).each do |method| class_eval <<-METHOD, __FILE__, __LINE__ + 1 diff --git a/actionpack/lib/action_dispatch/request/utils.rb b/actionpack/lib/action_dispatch/request/utils.rb index 9d4f1aa3c5..1c9371d89c 100644 --- a/actionpack/lib/action_dispatch/request/utils.rb +++ b/actionpack/lib/action_dispatch/request/utils.rb @@ -16,10 +16,6 @@ module ActionDispatch when Array v.grep(Hash) { |x| deep_munge(x, keys) } v.compact! - if v.empty? - hash[k] = nil - ActiveSupport::Notifications.instrument("deep_munge.action_controller", keys: keys) - end when Hash deep_munge(v, keys) end diff --git a/actionpack/test/dispatch/request/json_params_parsing_test.rb b/actionpack/test/dispatch/request/json_params_parsing_test.rb index c609075e6b..b765a13fa1 100644 --- a/actionpack/test/dispatch/request/json_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/json_params_parsing_test.rb @@ -39,7 +39,7 @@ class JsonParamsParsingTest < ActionDispatch::IntegrationTest test "nils are stripped from collections" do assert_parses( - {"person" => nil}, + {"person" => []}, "{\"person\":[null]}", { 'CONTENT_TYPE' => 'application/json' } ) assert_parses( @@ -47,7 +47,7 @@ class JsonParamsParsingTest < ActionDispatch::IntegrationTest "{\"person\":[\"foo\",null]}", { 'CONTENT_TYPE' => 'application/json' } ) assert_parses( - {"person" => nil}, + {"person" => []}, "{\"person\":[null, null]}", { 'CONTENT_TYPE' => 'application/json' } ) end diff --git a/actionpack/test/dispatch/request/query_string_parsing_test.rb b/actionpack/test/dispatch/request/query_string_parsing_test.rb index 4e99c26e03..50daafbb54 100644 --- a/actionpack/test/dispatch/request/query_string_parsing_test.rb +++ b/actionpack/test/dispatch/request/query_string_parsing_test.rb @@ -95,8 +95,8 @@ class QueryStringParsingTest < ActionDispatch::IntegrationTest 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" => [] }}}, "action[foo][bar][]") + assert_parses({"action" => {"foo" => [] }}, "action[foo][]") assert_parses({"action"=>{"foo"=>[{"bar"=>nil}]}}, "action[foo][][bar]") end diff --git a/guides/source/action_controller_overview.md b/guides/source/action_controller_overview.md index 4e36a62583..57546da389 100644 --- a/guides/source/action_controller_overview.md +++ b/guides/source/action_controller_overview.md @@ -112,8 +112,8 @@ NOTE: The actual URL in this example will be encoded as "/clients?ids%5b%5d=1&id The value of `params[:ids]` will now be `["1", "2", "3"]`. Note that parameter values are always strings; Rails makes no attempt to guess or cast the type. -NOTE: Values such as `[]`, `[nil]` or `[nil, nil, ...]` in `params` are replaced -with `nil` for security reasons by default. See [Security Guide](security.html#unsafe-query-generation) +NOTE: Values such as `[nil]` or `[nil, nil, ...]` in `params` are replaced +with `[]` for security reasons by default. See [Security Guide](security.html#unsafe-query-generation) for more information. To send a hash you include the key name inside the brackets: diff --git a/guides/source/security.md b/guides/source/security.md index b1c5b22338..b3869b1ba5 100644 --- a/guides/source/security.md +++ b/guides/source/security.md @@ -942,7 +942,7 @@ unless params[:token].nil? end ``` -When `params[:token]` is one of: `[]`, `[nil]`, `[nil, nil, ...]` or +When `params[:token]` is one of: `[nil]`, `[nil, nil, ...]` or `['foo', nil]` it will bypass the test for `nil`, but `IS NULL` or `IN ('foo', NULL)` where clauses still will be added to the SQL query. @@ -953,9 +953,9 @@ request: | JSON | Parameters | |-----------------------------------|--------------------------| | `{ "person": null }` | `{ :person => nil }` | -| `{ "person": [] }` | `{ :person => nil }` | -| `{ "person": [null] }` | `{ :person => nil }` | -| `{ "person": [null, null, ...] }` | `{ :person => nil }` | +| `{ "person": [] }` | `{ :person => [] }` | +| `{ "person": [null] }` | `{ :person => [] }` | +| `{ "person": [null, null, ...] }` | `{ :person => [] }` | | `{ "person": ["foo", null] }` | `{ :person => ["foo"] }` | It is possible to return to old behaviour and disable `deep_munge` configuring |