14

let us say I have a list page of users and you can sort by the different columns, when clicking 'email' it will pass sort_by=email sort_direction=asc or desc

sort_by = "email" # really params[:sort_by]
sort_direction = "asc" # really params[:sort_direction]
User.order("#{sort_by} #{sort_direction}")
# SELECT "users".* FROM "users" ORDER BY email asc

so that works as expected, however if we change the sort_by

sort_by = "email; DELETE from users; --"
User.order("#{sort_by} #{sort_direction}")
# SELECT "users".* FROM "users" ORDER BY email; DELETE from users; -- asc

now we have no more users :(

I can manually build a whitelist of valid sort_by and compare params[:sort_by] to that, but was hoping there is some built in way to handle this kind of thing

house9
  • 20,359
  • 8
  • 55
  • 61

1 Answers1

26

Ryan Bates' method:

in your controller:

def index
  @users = User.order(sort_by + " " + direction)
end

private
  def sort_by
    %w{email name}.include?(params[:sort_by]) ? params[:sort_by] : 'name'
  end

  def direction
    %w{asc desc}.include?(params[:direction]) ? params[:direction] : 'asc'
  end

Essentially you're making a whitelist, but it's easy to do and insusceptible to injection.

bricker
  • 8,911
  • 2
  • 44
  • 54
  • 7
    And if you don't care what columns they can sort on, you can replace `%w{email name}` with `User.column_names` – Philip Hallstrom Oct 14 '11 at 18:23
  • Looking at this, I think I must have adopted my code from Ryan's because I wrote it a long time ago. For me, I think it works and is safe, but I'm not that good, and recently, I noticed that brakeman seems to be complaining about a potential sql injection. Assuming right that my verison is OK, and I'm not the only one, is there any way to convince brakeman I'm OK? – codenoob Feb 21 '16 at 01:08