Read more

Security issues with hash conditions in Rails 2 and Rails 3

Arne Hartherz
October 05, 2010Software engineer at makandra GmbH

Find conditions for scopes can be given either as an array (:conditions => ['state = ?', 'draft']) or a hash (:conditions => { 'state' => 'draft' }). The later is nicer to read, but has horrible security implications in some versions of Ruby on Rails.

Affected versions

Version Affected? Remedy
2.3.18 yes Use chain_safely workaround
3.0.20 no
3.1.x ???
3.2.22 yes Use Rails LTS 3.2 Show archive.org snapshot with hardened configuration
4.0.x ??? ???
4.2.3 no

Example

Illustration money motivation

Opscomplete powered by makandra brand

Save money by migrating from AWS to our fully managed hosting in Germany.

  • Trusted by over 100 customers
  • Ready to use with Ruby, Node.js, PHP
  • Proactive management by operations experts
Read more Show archive.org snapshot

When you chain two scopes with hash conditions on the same attribute, the second scope will overwrite (and not join!) conditions from the first scope:

class Contract < ActiveRecord::Base
  named_scope :with_region, proc { |region_id| { :conditions => { :region_id => region_id } } }
end

>> Contract.with_region(5)
=> SELECT * FROM `contracts` WHERE (`contracts`.`region_id` = 5)

>> Contract.with_region(5).with_region(7)
=> SELECT * FROM `contracts` WHERE (`contracts`.`region_id` = 7)

Note how the last query did not select on region_id = 5 AND region_id = 7 as expected.

Unfortunately we cannot use array-conditions all the time to work around this. The reason is that when you build a record from a scope with array conditions, the resulting record will not have the attributes that defined that scope, and this behavior can be quite useful.

Because you cannot have the best from both worlds you should understand this article and make informed choices about how to define your conditions. Also see the Best Practice section below.

The problem in detail

Consider this Rails class and the following articles:

class Article < ActiveRecord::Base
  named_scope :available, :conditions => { :state => [ 'unassigned', 'draft' ] }
end

#<Article id: 1, title: "Cooking noodles", state: "unassigned">
#<Article id: 2, title: "How to rock", state: "draft">
#<Article id: 3, title: "Skip work", state: "deleted">

If you want to fetch all available articles, you would do something like this:

>> Article.available
# SELECT * FROM `articles` WHERE (`articles`.`state` IN ('unassigned','draft')) 
=> [ #<Article id: 1, title: "Cooking noodles", state: "unassigned">, #<Article id: 2, title: "How to rock", state: "draft"> ]

Let's say you wanted to include deleted articles. Removing the available scope is one way but you might also add another scope -- which results in an unexpected effect:

>> Article.available.scoped(:conditions => { :state => 'deleted' })
# SELECT * FROM `articles` WHERE ((`articles`.`state` = 'deleted'))
=> [ #<Article id: 3, title: "Skip work", state: "deleted"> ]

Here the conditions inside the last scope will overwrite any previous conditions on the state attribute; they will not be combined.

Note that the unexpected overwriting of conditions only happens when chaining scopes. Hash conditions are joined as expected when using find (but scope chains are not going away).

Security implications

This can be a security problem when you use scope to restrict what a controller can see and edit, but also allow the user to further filter that scope.

One possible scenario are filters defined through special patterns in a search field, like "state:draft".
If your controller parses HTTP parameters and adds such filters as conditions to your scope chain you must not supply those conditions as a hash (as seen above).
To be on the safe side, use SQL fragments which are never overridden:

>> Article.available.scoped(:conditions => [ 'articles.state = ?', 'deleted' ])
# SELECT * FROM `articles` WHERE ((articles.state = 'deleted') AND (`articles`.`state` IN ('unassigned','draft')))
=> []

Unfortunately you can no longer build a record from a scope with array conditions and expect the resulting record to have the attributes that defined that scope. This is why hash conditions are not going away.

Best practice

  • If possible, use an unaffected Rails version
  • Keep using hash conditions for reasons of elegance and the ability to build records from such scopes.
  • As soon as user input comes into play you should use our chain_safely pseudo-scope to turn a scope with hash conditions into a scope with array conditions.
Arne Hartherz
October 05, 2010Software engineer at makandra GmbH
Posted by Arne Hartherz to makandra dev (2010-10-05 19:14)