orm_adapter
orm_adapter copied to clipboard
ianwhite
ActiveRecord adapter bypasses strong_parameters
ActiveRecord 4.1.5 addressed a security vulnerability whereby parameters passed to where were not sanitized, so cases where where was used in conjunction with create could potentially allow injecting arbitrary attributes if the conditions hash was user-specified: https://github.com/rails/rails/commit/9456990b14ea77f3d489ad6b6a65af96f3cda9ef
orm_adapter's find_all method, because it uses conditions_to_fields and recreates a conditions hash from scratch, discards the original hash, which may have been a strong parameters hash, in which case that information is now lost.
This may not actually be an issue, since vulnerable code would need to be depending on both orm_adapter's and ActiveRecord's API (chaining create onto the return value of find_all), which defeats the purpose of using orm_adapter in the first place, but I thought it worth mentioning, since it's a somewhat surprising behaviour that's different from what ActiveRecord does.
I noticed this inconsistency in an override to a Devise controller that started erroring with ForbiddenAttributes when I updated to Rails 4.1.5. The original Devise controller used find_first and so bypassed strong parameters, while my version used Rails' where().first and threw the error.
Do you think it's worth updating orm_adapter to preserve the strong params information, either as a security concern or just for consistency of behaviour with the equivalent ActiveRecord APIs?
Hi Louis, thanks for writing this up.
I see no harm in changing conditions_to_fields to return an object of the same type as the conditions parameter, basically a dup, but with the association keys added, and the association objects removed. Do you think that would preserve the strong parameter information?
Yeah, I think a dup with a bit of extra massaging as you've described would do the trick. Let me see what I can put together, I'll send a PR (most likely tomorrow, it's pretty late here).