Hello All!
Is the ActiveRecord::Base.find() :conditions option is safe to SQL
Injections? Any comments on that?
it depends on correct usage
from ActiveRecord::Base
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 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, 3:28 pm, Thorsten M. [email protected]
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 May 6, 6:16 pm, Ar Chron [email protected] 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.