rails/rails — pull request #16924 by Sinjo (2014-09-15 11:30:09+00:00)
Title: Don't convert empty arrays to nils when deep munging params

This commit tweaks the behaviour of `deep_munge` in light of changes made to improve security in ActiveRecord.

Previously, when an empty array was passed as an argument to a `where` or `find_by` query, it would generate SQL with an `IS NULL` clause. This lead to vulnerabilities due to records being returned in cases where app code didn't expect it. These are documented in:

* [CVE-2012-2660](https://groups.google.com/forum/#!searchin/rubyonrails-security/deep_munge/rubyonrails-security/8SA-M3as7A8/Mr9fi9X4kNgJ),
* [CVE-2012-2694](https://groups.google.com/forum/#!searchin/rubyonrails-security/deep_munge/rubyonrails-security/jILZ34tAHF4/7x0hLH-o0-IJ)
* [CVE-2013-0155](https://groups.google.com/forum/#!searchin/rubyonrails-security/CVE-2012-2660/rubyonrails-security/c7jT-EeN9eI/L0u4e87zYGMJ)

Now (example using Rails 4.1.5 and Postgres 9.2.8), ActiveRecord generates a query like:

```
[1] pry(main)> User.find_by_email([])
---
User Load (29.3ms) SELECT "users".* FROM "users" WHERE 1=0 LIMIT 1
```

which never returns any rows.

This new behaviour makes it possible to change the behaviour of `deep_munge` in what seems like a preferable way. While `nil`s are still stripped from arrays in params, empty arrays won't be converted into nil. Apps would no longer need to work around this behaviour by re-parsing JSON if they want to distinguish between `nil` and `[]`.

Conveniently, this would fix rails/strong_parameters#192 as well.

I realise it wouldn't be appropriate to target this change at 4.2 this late in the day, but it seems like it would be a nice improvement for 4.3.