Rails Brakeman: SQL Injection for Arel

626 views Asked by At

I have the following code in my user_ransaker.rb file:

ransacker :new_donors do
      sql = %{(
              users.id IN (
                #{User.new_donor_sql}
              )
            )}
      Arel.sql(sql)
    end

On user.rb model:

def self.new_donor_sql
    part_1 = %{(
      SELECT distinct(user_id)
      FROM donations
    }
    part_1
end

I get the following Brakeman warning for above statement:

Confidence: High
Category: SQL Injection
Check: SQL
Message: Possible SQL injection
Code: Arel.sql("(\n users.id IN (\n #{User.new_donor_sql}\n)\n)")
File: app/models/concerns/user_ransackers.rb

Is this a valid error? If I used ActiveRecord to write the SQL statement, I could have used ? placeholder if I needed to interpolate values. I am not really sure how to fix this warning. If this is a valid warning, how do I remediate it?

1

There are 1 answers

2
max On

If you gonna Arel then do some relational algebra:

class User < ApplicationRecord
  def self.new_donor_sql
    arel_table.project(arel_table[:user_id]).distinct
  end
end
ransacker :new_donors do
  User.arel_table.then do |users|
    users.where(users[:id].in(User.new_donor_sql)).where_sql
  end
end

You could also just drop the class method:

ransacker :new_donors do
  User.arel_table.then do |users|
    subquery = users.project(users[:user_id]).distinct
    users.where(users[:id].in(subquery)).where_sql
  end
end