Rails 5 ActiveRecord Sanitized Order By

It may surprise you to find out that the default rails order function does not sanitize input. This can lead to potentially dangerous sql injections. Additionally, Rails 5 is starting to throw warnings when using order or sanitize_sql_for_order:

DEPRECATION WARNING: Dangerous query method (method whose arguments are used as raw SQL) called with non-attribute argument(s): "field(id, ?)". Non-attribute arguments will be disallowed in Rails 6.0. This method should not be called with user-provided values, such as request parameters or model attributes. Known-safe values can be passed by wrapping them in Arel.sql().

I don’t like users to be able to cause sql errors even with the worst input. I like clean logs. I went looking for someone who has solved this problem but didn’t find much. There was a decent sanitation function in a gist on github but it doesn’t work with joins. Here is a version of that function that operates cleanly when used on a joined query.

Add this file to config/initializers/sanitized_order.rb:

# Refactored from https://gist.github.com/TheKidCoder/9653073
class ActiveRecord::Relation
  def sanitized_order(order_by, direction = 'ASC')
    if order_by.include?('.')
      klass, column = order_by.split('.')
      unless ([model_name.plural.to_sym] + joins_values).include?(klass.pluralize.to_sym)
        raise "#{klass} unavailable in query"
      end
      klass = klass.singularize.classify.constantize
    else
      klass = self.klass
      column = order_by
    end

    raise "Column #{column} not found in #{klass.name}" unless klass.column_names.include?(column.to_s)
    raise 'Invalid direction value' unless %w[ASC DESC].include?(direction.upcase)

    order("#{order_by} #{direction.upcase}")
  end
end

Use it like this and feel free to pass in user supplied data:

Users.joins(:posts).sanitized_order('posts.id', 'DESC')

Beware though, a malicious user could use this to order by any column available in the model so you may need to add more protections to this depending on your use case.


Comments (3)

mcfoton
Wednesday, May 1, 2019

Hi James, thanks for sharing :) One question though — what if I have the model name stated in the order clause directly, is there a way to check no only joins_values, but kind-of from-values? Couldn’t find a method for that :(

 scope :ordered_by_city, ->(order) {sanitized_order("addresses.city", order)}

Address.ordered_by_city('ASC')
->> RuntimeError (addresses unavailable in query)

James Kiefer
In reply to mcfoton
Wednesday, May 1, 2019

Oh good catch! Your example shouldn’t fail. I’ve revised the article and the function.

mcfoton
In reply to mcfoton
Sunday, May 12, 2019

Thanks! I’ve found some more edge cases. I’ll try to fix them and will update you a bit later. For now these are:

  1. When model is joined using a single form: joins(:state) it will not match against klass.pluralize.to_sym
  2. When join is done using syntax of joins(model1: :model2) it should unwrap the hash from joins_values

P.S. I believe this is a valuable snippet we should polish so you can post it all over stackoverflow :)

Thank you

Your comment has been submitted and will be published once it has been approved.

OOPS!

Your comment has not been submitted. Please go back and try again. Thank You!

If this error persists, please open an issue by clicking here.

Say something