Forum: Ruby on Rails ActiveRecord::Base#find is SQL injection free???

Hello All!
   Is the ActiveRecord::Base.find() :conditions option is safe to SQL
Injections? Any comments on that?

it depends on correct usage

from http://api.rubyonrails.org/classes/ActiveRecord/Base.html

Conditions can either be specified as a string, array, or hash 
representing the WHERE-part of an SQL statement. The array form is to be 
used when the condition input is tainted and requires sanitization. The 
string form can be used for statements that don‘t involve tainted data. 
The hash form works much like the array form, except only equality and 
range is possible. Examples:

  class User < ActiveRecord::Base
    def self.authenticate_unsafely(user_name, password)
      find(:first, :conditions => "user_name = '#{user_name}' AND 
password = '#{password}'")
    end

    def self.authenticate_safely(user_name, password)
      find(:first, :conditions => [ "user_name = ? AND password = ?", 
user_name, password ])
    end

    def self.authenticate_safely_simply(user_name, password)
      find(:first, :conditions => { :user_name => user_name, :password 
=> password })
    end
  end

The authenticate_unsafely method inserts the parameters directly into 
the query and is thus susceptible to SQL-injection attacks if the 
user_name and password parameters come directly from an HTTP request. 
The authenticate_safely and authenticate_safely_simply both will 
sanitize the user_name and password before inserting them in the query, 
which will ensure that an attacker can‘t escape the query and fake the 
login (or worse).

On 6 May 2008, at 11:18, MohsinHijazee wrote:

>
> Hello All!
>   Is the ActiveRecord::Base.find() :conditions option is safe to SQL
> Injections? Any comments on that?

Yes it is, as long as you use it in the intended way (ie :conditions
=> {:name => 'foo}, :conditions => ['name = ?', 'foo'] etc...).
You can break it if you try, eg :conditions => "#{some_string}"

Fred

On May 6, 3:28 pm, Thorsten Mueller <rails-mailing-l...@andreas-s.net>
wrote:
>
>
> sanitize the user_name and password before inserting them in the query,
> which will ensure that an attacker can‘t escape the query and fake the
> login (or worse).
>
Thank you very much for this useful post!
Well, my situation is worst then that.
My WHOLE condition comes from the user like:
params[:conditions] = "user_name='Raphael' AND age > 20"
So the essence is that any condition can be specified.

And in my controller, I do this:
users = User.find(:all, :conditions => params[:conditions])
return users

is this ok??? If not, then any workaround?

On 6 May 2008, at 12:00, MohsinHijazee wrote:
> is this ok??? If not, then any workaround?
That is not a good idea. It's not sql injection free because what
you're doing is allowing people to inject sql. In fact you're not just
allowing it, you're depending on it.
It also sounds like a pretty horrific piece of UI.
Fred

MohsinHijazee wrote:

> Thank you very much for this useful post!
> Well, my situation is worst then that.
> My WHOLE condition comes from the user like:
> params[:conditions] = "user_name='Raphael' AND age > 20"

Ack!!!

The only safe way to do that (that occurs to me at the moment) is to 
strip apart the conditions you receive into a list of field / value 
pairs, validate each field against what should be allowed for the model 
(discarding those that don't reference valid fields, or reference fields 
the user shouldn't know about), then construct a conditions statement 
that still allows rails to do its thing with either of the "safely" 
methods shown above.

On May 6, 6:16 pm, Ar Chron <rails-mailing-l...@andreas-s.net> wrote:
> pairs, validate each field against what should be allowed for the model
> (discarding those that don't reference valid fields, or reference fields
> the user shouldn't know about), then construct a conditions statement
> that still allows rails to do its thing with either of the "safely"
> methods shown above.
yes you are right.  I also thought about it that we need to verify the
incoming fields on bases of key value pairs and then remove unsafe
parts perhaps. This is not part of the UI, its basically a requirement
for the REST API where it should be possible for the consumer of the
API to specify the records under certain conditions.

I have tested: find(params[:id]) by submitting:
a' or 't'='t
or
1; DROP TABLE super_cool_table;

and it is perfectly safe, the generated SQL query did not pick up the 
injection.