0

In my Rails 3.2 app, Brakeman 1.8.3 raises a High confidence SQL injection warning for the following code in a model:

micropost.rb

def self.from_users_followed_by(user)
  followed_user_ids = Relationship.select(:followed_id).
                      where("follower_id = :user_id").
                      to_sql
  where("user_id IN (#{followed_user_ids}) OR user_id = :user_id",
        user_id: user.id)
end

However, when I change the code to not use Arel syntax, no warning is raised:

def self.from_users_followed_by(user)
  followed_user_ids = "SELECT followed_id FROM relationships
                       WHERE follower_id = :user_id"
  where("user_id IN (#{followed_user_ids}) OR user_id = :user_id",
        user_id: user.id)
end

Is this a false positive, or something to do with Arel syntax or the to_sql method...? I don't understand what the difference is between the actual code that gets executed in the two examples that would warrant the warning.

Paul Fioravanti
  • 16,423
  • 7
  • 71
  • 122

1 Answers1

3

It's a false positive.

In this situation, Brakeman knows Relationship is a model, and that select and where are query methods. So it assumes Relationship.select(...).where(...).to_sql is a record attribute (and potentially dangerous). It shouldn't, though, since to_sql just generates the SQL code for the query as you mentioned. I'll fix this.

The second version of course does not warn because you are interpolating a string literal.

Justin
  • 1,561
  • 10
  • 12