5 ms·

Props to OP for finding the vulnerability without knowing Ruby on Rails.

• qyv 10y ago
  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#ordering http://guides.rubyonrails.org/active_record_querying.html#or...
  • andy_ppp 10y ago
    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.
    • grey-area 10y ago
      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 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.
      • true_religion 10y ago
        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.
        • smithd98 10y ago
          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.