I know how to avoid SQL injection attacks when you use :conditions User.find :first, :conditions => ["login=?", params[:username]] but how about with :order, :limit or :group? # uh-oh...spaghetti-oh User.find :first, :order => "login; delete from users; select * from users" Pat --~--~---------~--~----~------------~-------~--~----~ 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 Oct 15, 2007, at 14:08 , Pat Maddox wrote:> I know how to avoid SQL injection attacks when you use :conditions > User.find :first, :conditions => ["login=?", params[:username]] > > but how about with :order, :limit or :group?I assume you''re permitting, for example, user-specified ordering? Unless you''re permitting users to modify the DDL, I think your best bet is to interpret their request though some parameter and build the query rather than interpolate their input directly. You might also look to see if the ActiveRecord adapter for the database you''re using has an equivalent to PostgreSQL''s quote_ident function. I don''t know if the PostgreSQL adapter exposes that functionality directly though. Michael Glaesemann grzm seespotcode net --~--~---------~--~----~------------~-------~--~----~ 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 run in to this a few times as well. If you want to simply allow
them to sort on the columns in a table, ascending / descending, then
this code makes it safe:
sort_col = User.column_names.include? params[:sort_col] ?
params[:sort_col] : ''created_at'' #or whatever default you want
sort_dir = params[:sort_dir].downcase == ''asc'' ?
''asc'' : ''desc''
User.find :first, :order => "#{sort_col} #{sort_dir}"
You can easily create a library function to use this elsewhere. Hope
this helps.
-Bill
Pat Maddox wrote:> I know how to avoid SQL injection attacks when you use :conditions
> User.find :first, :conditions => ["login=?",
params[:username]]
>
> but how about with :order, :limit or :group?
>
> # uh-oh...spaghetti-oh
> User.find :first, :order => "login; delete from users; select *
from users"
>
> Pat
>
> >
>
--
Sincerely,
William Pratt
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---
Since you probably have a view that allows only specific ordering
columns and directions, I would suggest something like:
allowed_directions = [ "asc", "desc" ]
allowed_columns = [ "posting_date", "sender",
"recipient" ]
sort_column = "default-column-name"
if allowed_columns.include?(params[:sortcolumn])
sort_column = params[:sortcolumn]
end
sort_direction = "asc"
if allowed_directions.include?(params[:sortdirection])
sort_direction = params[:sortcolumn]
end
and then use the sort column itself. This sanitizes user input by
following the rule of allowing specific actions and disallowing all
others.
I _think_ you can also do:
:order => [ ":column :direction", { :column =>
params[:sortcolumn],
:direction => params[:sortdirection] }]
but I''m not certain. I like my way better -- it''s less
trusting of magic.
--Michael
--~--~---------~--~----~------------~-------~--~----~
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 10/15/07, Michael Graff <skan.gryphon-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> > Since you probably have a view that allows only specific ordering > columns and directions, I would suggest something like: > > allowed_directions = [ "asc", "desc" ] > allowed_columns = [ "posting_date", "sender", "recipient" ] > > sort_column = "default-column-name" > if allowed_columns.include?(params[:sortcolumn]) > sort_column = params[:sortcolumn] > end > sort_direction = "asc" > if allowed_directions.include?(params[:sortdirection]) > sort_direction = params[:sortcolumn] > end > > and then use the sort column itself. This sanitizes user input by > following the rule of allowing specific actions and disallowing all > others. > > I _think_ you can also do: > > :order => [ ":column :direction", { :column => params[:sortcolumn], > :direction => params[:sortdirection] }] > > but I''m not certain. I like my way better -- it''s less trusting of magic.Yeah, my initial concern was that there are a lot of sort / group variations and they will change over time. I think what I''ll end up doing is writing a little helper object to offload that. So then I have something like MyModel.find :all, SanitizedOptions.new(:order => params[:order], :limit => params[:limit], :group => params[:group]) Implementation will be simple - just check to see if the passed in option is in a particular array, like you said. Then the finder doesn''t have to deal with that stuff, I express that this is a potentially dangerous method, and it''s obvious where changes to the allowed options should go. Thanks for the input everybody. Pat --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
You might find this of use:>> User.column_names=> ["id", "login", "email", "crypted_password", "salt", "password_reset_code", "activation_code", "activated_at", "site_admin", "created_at", "updated_at", "lock_version", "time_zone"] This should let you make a generic function as a module, and then just mix it into each of your models. I think. :) --Michael --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Take a look at my post above, that is exactly what I am doing :) Michael Graff wrote:> You might find this of use: > > >>> User.column_names >>> > => ["id", "login", "email", "crypted_password", "salt", > "password_reset_code", "activation_code", "activated_at", > "site_admin", "created_at", "updated_at", "lock_version", "time_zone"] > > This should let you make a generic function as a module, and then just > mix it into each of your models. I think. :) > > --Michael > > > >-- Sincerely, William Pratt --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---