Hello All! Is the ActiveRecord::Base.find() :conditions option is safe to SQL Injections? Any comments on that? --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
Thorsten Mueller
2008-May-06 10:28 UTC
Re: ActiveRecord::Base#find is SQL injection free???
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). -- Posted via http://www.ruby-forum.com/. --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
Frederick Cheung
2008-May-06 10:40 UTC
Re: ActiveRecord::Base#find is SQL injection free???
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 --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
On May 6, 3:28 pm, Thorsten Mueller <rails-mailing-l...-ARtvInVfO7ksV2N9l4h3zg@public.gmane.org> wrote:> 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?> -- > Posted viahttp://www.ruby-forum.com/.--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
Frederick Cheung
2008-May-06 12:30 UTC
Re: ActiveRecord::Base#find is SQL injection free???
On 6 May 2008, at 12:00, 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" > 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 --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
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. -- Posted via http://www.ruby-forum.com/. --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
On May 6, 6:16 pm, Ar Chron <rails-mailing-l...-ARtvInVfO7ksV2N9l4h3zg@public.gmane.org> wrote:> 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.> > -- > Posted viahttp://www.ruby-forum.com/.--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
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. -- Posted via http://www.ruby-forum.com/. --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---