False positives SQL injections

[aglushkov]

Background

Brakeman version: brakeman 5.0.0 Rails version: Rails 6.0.3.4 Ruby version: ruby 2.7.2p137 (2020-10-01 revision 5445e04352) [x86_64-linux]

Issue

There are 4 examples that I believe free from any possible sql injection, but brakeman show warnings

class BrakemanIssues
  # Issue with model name
  def issue1
    votable_type = User.name
    Arel.sql("votes.votable_type = '#{votable_type}'")
  end

  # Issue with variable selected from some constant list
  def issue2_constant_hash_value(role_name)
    roles = { admin: 1, moderator: 2 }.freeze
    role = roles.fetch(role_name)
    Arel.sql("role = '#{role}'")
  end

  # Issue with already escaped variable
  def issue3(query)
    query = ActiveRecord::Base.sanitize_sql_like(query) # escaped variable
    Arel.sql("name ILIKE '%#{query}%'")
  end

  # Issue with constant date
  def issue4
    date = 30.days.ago
    Arel.sql("created_at > '#{date}'")
  end
end

== Warnings ==

Confidence: High
Category: SQL Injection
Check: SQL
Message: Possible SQL injection
Code: Arel.sql("votes.votable_type = '#{User.name}'")
File: app/queries/brakeman.rb
Line: 5

Confidence: Medium
Category: SQL Injection
Check: SQL
Message: Possible SQL injection
Code: Arel.sql("role = '#{{ :admin => 1, :moderator => 2 }.fetch(role_name)}'")
File: app/queries/brakeman.rb
Line: 12

Confidence: Medium
Category: SQL Injection
Check: SQL
Message: Possible SQL injection
Code: Arel.sql("name ILIKE '%#{ActiveRecord::Base.sanitize_sql_like(query)}%'")
File: app/queries/brakeman.rb
Line: 18

Confidence: Medium
Category: SQL Injection
Check: SQL
Message: Possible SQL injection
Code: Arel.sql("created_at > '#{30.days.ago}'")
File: app/queries/brakeman.rb
Line: 24

[aglushkov]

There are really possible SQL injection is issue3, sorry. I've used sanitize_sql to fix all this warnings, but it would be great if Brakeman don't argue on class names or constant values

[presidentbeef]

I agree - all of these are fixable.

[natematykiewicz]

@presidentbeef I just want to make sure everyone's on the same page. Example 3 here is open to SQL Injection. sanitize_sql_like simply escapes _ and %.

https://github.com/rails/rails/blob/83217025a171593547d1268651b446d3533e2019/activerecord/lib/active_record/sanitization.rb#L94-L111

[ghost]

@presidentbeef you ignored almost all sanitize methods. However, you missed method sanitize_sql_for_order from the document https://api.rubyonrails.org/classes/ActiveRecord/Sanitization/ClassMethods.html#method-i-sanitize_sql_for_order