CODEWITHSUNDEEP
ruby-on-railsactiverecordsql-injection
user3494179
user3494179

Reputation: 373

Is this vulnerable to sql injection or not?

While looking for a solution to my problem, i found this topic:

Rails and Arel and Scopes - simplify multiple OR's on the same table/field match

One of the answers is this:

fields = ["name", "email", "phone", "business_name", "doc_id"]
filter = fields.map { |field| "lower(#{field}) LIKE '#{term.downcase}'" }.join(' OR ')
@customers = Customer.where(filter)

But isn't this vulnarable to sql injections if term is a param from user input? If so, how can I make it sql injection proof? Will sanitize_sql_like fix the sql injection issue? So like this:

filter = fields.map { |field| "lower(#{field}) LIKE '#{sanitize_sql_like(term.downcase)}'" }.join(' OR ')
@customers = Customer.where(filter)

Update: Is this a solution?

fields = ["name", "email", "phone", "business_name", "doc_id"]
filter = fields.map { |field| "lower(#{field}) LIKE :query" }.join(' OR ')
@customers = Customer.where(filter, query: "%#{sanitize_sql_like(term.downcase)}%")

Upvotes: 1

Views: 542

Answers (1)

max
max

Reputation: 102036

Any situation where you are interpolating user input into a SQL string is a potential SQL injection vulnerability. Escaping the input (which is what sanitize_sql_like does) can mitigate it somewhat but its always better to use bind variables to pass the user input separately to the database so that it cannot be missinterpreted as actual commands.

# bad
User.where("email like %#{params[:email]}%")

# good
User.where("email like ?", "%#{params[:email]}%")

You also do not need to manually construct that huge SQL string. Instead just create an array of ActiveRecord::Relations and join them together with the .or method.

fields = %w{ name email phone business_name doc_id }
filters = fields.map do |field|
  Customer.where("lower(#{field}) LIKE :term", term: term.downcase)
end
@customers = filters.drop(1).inject(filters.first) do |memo, scope|
  memo.or(scope)
end

This can look somewhat daunting but its really just a fancy pants version of:

Customer.where("lower(name) LIKE :term", term: term.downcase)
        .or(
          Customer.where("lower(email) LIKE :term", term: term.downcase)
        )
        .or(
          Customer.where("lower(phone) LIKE :term", term: term.downcase)
        )
        # ...

Upvotes: 3

Related Questions