SQL injection with :order, :limit, :group

Rails
1

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

2

On Oct 15, 2007, at 14:08 , Pat M. 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 G.
grzm seespotcode net

3

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 M. wrote:

–
Sincerely,

William P.

4

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

5

On 10/15/07, Michael G. [email protected] wrote:

end

: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

6

Take a look at my post above, that is exactly what I am doing :slight_smile:

Michael G. wrote:

mix it into each of your models. I think. :slight_smile:

–Michael

–
Sincerely,

William P.

7

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. :slight_smile:

–Michael