ivan
ivan

Reputation: 6322

Should this be considered an SQL injection risk?

Say I have a Rails app with 3 models: Job, Contractor, Bid. When a contractor bids on a job, a bid is created with the appropriate contractor_id and job_id. Now I'd like to define a Job scope that, given a contractor, returns the jobs they have not yet bid on:

class Job < ActiveRecord::Base
  scope :not_bid_on_by, -> (contractor) do
    joins(%{
      LEFT JOIN bids ON bids.job_id = jobs.id
      AND bids.contractor_id = #{ contractor.id }
    }).where(bids: { id: nil })
  end
end

Despite the fact that a value is interpolated directly into the joins string, I can't think of a case where this example would be vulnerable to SQL injection, since it's only interpolating an id field from the given argument. I know it's bad practice in general, but this seems like a benign instance.

If I'm wrong and this is a security risk, is there a safer way to do it? ActiveRecord doesn't seem to have a way of safely interpolating into the joins string like it does for where (with ? or named parameters).

Upvotes: 0

Views: 31

Answers (1)

Sean Huber
Sean Huber

Reputation: 3985

Try using ActiveRecord's sanitize_sql_array method: http://api.rubyonrails.org/classes/ActiveRecord/Sanitization/ClassMethods.html#method-i-sanitize_sql_array

Example:

class Job < ActiveRecord::Base
  scope :not_bid_on_by, -> (contractor) do
    joins( sanitize_sql_array [%{
      LEFT JOIN bids ON bids.job_id = jobs.id
      AND bids.contractor_id = ?
    }, contractor.id]).where(bids: { id: nil })
  end
end

Upvotes: 1

Related Questions