Sql locking warning
My model has a scope:
scope :assigned_to_user, ->(user) {
task_table = UserTask.table_name
joins("INNER JOIN #{task_table}
ON #{task_table}.user_id = #{user.id}
AND (#{task_table}.type_id = #{table_name}.type_id)
AND (#{task_table}.manager_id = #{table_name}.manager_id)
")
}
So after running the brakeman report, I get the following warning:
assigned_to_user | SQL Injection | Possible
So, I tried the following:
scope :assigned_to_user, ->(user) {
task_table = UserTask.table_name
joins(ActiveRecord::Base::sanitize("INNER JOIN #{task_table}
ON #{task_table}.user_id = #{user.id}
AND (#{task_table}.type_id = #{table_name}.type_id)
AND (#{task_table}.manager_id = #{table_name}.manager_id)
"))
}
This doesn't work for me because it adds '
(apostrophe) to the front and back of the sql. So when I use this as part of a query that returns some results and I apply that scope, it generates invalid sql.
I also tried this:
scope :assigned_to_user, ->(user) {
task_table = UserTask.table_name
joins("INNER JOIN #{task_table}
ON #{task_table}.user_id = ?
AND (#{task_table}.type_id = #{table_name}.type_id)
AND (#{task_table}.manager_id = #{table_name}.manager_id)
", user.id)
}
Doesn't even create an instruction. And tried a couple of other things that didn't work and are not even worth mentioning. Does anyone know how to fix this?
source to share
After doing some research here, I will use. There is a method called sanitize_sql_array
( ref ), you can use it to execute statements by passing an sql string and replacement values to it:
sanitize_sql_array(['user_id = :user_id', user_id: 5])
# => "user_id = 5"
If we pass a table name to this method, it will remove it as well, but apply the quote
object method ActiveRecord::Base.connection
to the value that is used to remove variables, but not the table names. Maybe it will work sometimes, but I failed when I was using PostrgreSQL because the method quote
uses single quotes, but PostgreSQL requires double quotes for table names.
sanitize_sql_array([
'INNER JOIN :table_name ON :table_name.user_id = :user_id',
{ table_name: 'users', user_id: 5 }
])
# => "INNER JOIN 'users' ON 'users'.user_id = 5"
connection
the object also has a method quote_table_name
that can be separately applied to table names to make sure they are escaped + use sanitize_sql_array
for user id.
scope :assigned_to_user, -> (user) {
task_table = connection.quote_table_name(UserTask.table_name)
current_table = connection.quote_table_name(table_name)
sanitized_sql = sanitize_sql_array([
"INNER JOIN #{task_table}
ON #{task_table}.user_id = :user_id
AND (#{task_table}.type_id = #{current_table}.type_id)
AND (#{task_table}.manager_id = #{current_table}.manager_id)",
{ user_id: user.id }
])
joins(sanitized_sql)
}
Or you could just use sanitize
in user.id
instead of wrapping everything in a method call sanitize_sql_array
( #{sanitize(user.id)}
).
By the way, Brakeman won't show any warnings yet because the request has been moved to a variable. Brakeman literally parses your code as it is and is unaware of the variable and its contents. So this is all just to make sure it slips away.
Just to close Brakeman, you can simply move the query to a variable:
scope :assigned_to_user, -> (user) {
task_table = UserTask.table_name
query = "INNER JOIN #{task_table}
ON #{task_table}.user_id = #{user.id}
AND (#{task_table}.type_id = #{table_name}.type_id)
AND (#{task_table}.manager_id = #{table_name}.manager_id)"
joins(query)
}
source to share