How to fix sql injection in order rails query

user_1958669 0
Viewed 923

Im new to ruby and followed this blog to fix sql injection bug but my query is giving error .

Original query :

class Car < ActiveRecord::Base
  ...
  has_one :Driver, lambda {
     where(status: PASSENGER_STATUS, connected_number: [phone, mobile])
    .order("FIELD (`classDummy`.`status`, #{PASSENGER_STATUS.join(', ')}")
  }, class_name: :classDummy

Whereas , PASSENGER_STATUS is ( in other class )

PASSENGER_STATUS = [
  'employed','temporary'
  ].freeze

SQL INJECTION fix I did

  order("FIELD (`classDummy`.`status`, ? )", PASSENGER_STATUS.join(', '))

But this is throwing exception while executing query.

Exception that Im getting :

ActiveRecord::StatementInvalid: Mysql2::Error: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '?,

sql-injection ruby-on-rails ruby
2 Answers

As Geoffroy already pointed out, in your specific example there is no risk for a SQL injection because the input data is under your control.

But nevertheless, you can find the sanitize_sql_for_order in the Rails Docs. And the example from the documentation matches exactly your use-case:

sanitize_sql_for_order(condition)

Accepts an array, or string of SQL conditions and sanitizes them into a valid SQL fragment for an ORDER clause.

sanitize_sql_for_order(["field(id, ?)", [1,3,2]])
# => "field(id, 1,3,2)"

sanitize_sql_for_order("id ASC")
# => "id ASC"

Using this method you can write your association like this:

has_one :Driver, lambda {
     where(status: PASSENGER_STATUS, connected_number: [phone, mobile])
       .order(sanitize_sql_for_order(["FIELD(`classDummy`.`status`, ?)", PASSENGER_STATUS])
}, class_name: :classDummy
user_24833130

The blog your read is about handling user input, in which case it's really important to prevent sql injecton.

In:

order("FIELD (`classDummy`.`status`, #{PASSENGER_STATUS.join(', ')})")

You are injection code in the SQL query but it's code you control and know precisely what it is, so there's no risk of SQL injection.

Now about your problem, order does not replace the parameter ? while generating the query, it simply does not work like where.

In other words: use the code you were using before, it worked perfectly fine and is not subject to SQL injection (as long as you control what PASSENGER_STATUS is)

Check the logs to see the SQL query sent to the server, you'll understand better what's going on

user_6103510
Related