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
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
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:
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
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 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
Take a look at my post above, that is exactly what I am doing
Michael Graff wrote: