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).
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
it depends on correct usage
fromhttp://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).
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?
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.
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.
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.