Skip to content

Commit

Permalink
Ignore calls with number targets in SQL
Browse files Browse the repository at this point in the history
e.g. `20.days.ago`

Part of #1571
  • Loading branch information
presidentbeef committed May 2, 2021
1 parent a0227fc commit 37b40dc
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 1 deletion.
15 changes: 14 additions & 1 deletion lib/brakeman/checks/check_sql.rb
Expand Up @@ -592,7 +592,8 @@ def safe_value? exp
IGNORE_METHODS_IN_SQL.include? exp.method or
quote_call? exp or
arel? exp or
exp.method.to_s.end_with? "_id"
exp.method.to_s.end_with? "_id" or
number_target? exp
end
when :if
safe_value? exp.then_clause and safe_value? exp.else_clause
Expand Down Expand Up @@ -695,4 +696,16 @@ def connect_call? result
active_record_models.include? klass
end
end

def number_target? exp
return unless call? exp

if number? exp.target
true
elsif call? exp.target
number_target? exp.target
else
false
end
end
end
5 changes: 5 additions & 0 deletions test/apps/rails6/app/models/group.rb
Expand Up @@ -2,4 +2,9 @@ class Group < ApplicationRecord
def uuid_in_sql
ActiveRecord::Base.connection.exec_query("select * where x = #{User.uuid}")
end

def date_in_sql
date = 30.days.ago
Arel.sql("created_at > '#{date}'")
end
end
13 changes: 13 additions & 0 deletions test/tests/rails6.rb
Expand Up @@ -121,6 +121,19 @@ def test_sql_injection_uuid_false_positive
:user_input => s(:call, s(:const, :User), :uuid)
end

def test_sql_injection_date_integer_target_false_positive
assert_no_warning :type => :warning,
:warning_code => 0,
:fingerprint => "5ec829ba8790c01a39faf6788f0754d39879a6e68a9de8804c6f25ac9c2f1ee6",
:warning_type => "SQL Injection",
:line => 8,
:message => /^Possible\ SQL\ injection/,
:confidence => 1,
:relative_path => "app/models/group.rb",
:code => s(:call, s(:const, :Arel), :sql, s(:dstr, "created_at > '", s(:evstr, s(:call, s(:call, s(:lit, 30), :days), :ago)), s(:str, "'"))),
:user_input => s(:call, s(:call, s(:lit, 30), :days), :ago)
end

def test_cross_site_scripting_sanity
assert_warning :type => :template,
:warning_code => 2,
Expand Down

0 comments on commit 37b40dc

Please sign in to comment.