Hacker News Reader

Comment by qyv | Hacker News Reader

Interesting that this is a standard use-case for ActiveRecord [0] and does not include any built-in protection against SQL injection. You can pass a hash of symbols into the method, but there is no safe way to pass string arguments with sanitizing them yourself.

[0] http://guides.rubyonrails.org/active_record_querying.html#or...

View on HN

God, IMO this should be reported as a bug in rails; in my opinion it ActiveRecord should never accept strings for this without parsing them.

This is probably incredibly common for things like order params in many web apps and also relies on silly conversion rules in MySQL. Any time you see order=name in params you should be suspicious. Better to use numeric enums. There are similar problems with column names, this site has some great examples which apply to many ohms, not just rails:

http://rails-sqli.org

The lesson is you should always strongly assert the type of user input (where you can, convert to known good values which is even better), and never magically convert types as rails/ MySQL does here. It's less convenient but safer.

Django for example will throw an error if an ordering parameter does not correspond to a database column.

If you have to specify all the columns of a table in Rails, then ActiveRecord ought to be able to restrict the order params to only column names.

I imagine some ORMs don't have this so people can do order by computed columns like "order by assets - liabilities"

To your point it probably makes sense to have an order method that verifies column names and an order_raw method that takes a string so the caller understands the risk, while protecting the usual case.

Reply on news.ycombinator.com