diff --git a/lib/brakeman/checks/check_sql.rb b/lib/brakeman/checks/check_sql.rb index d62f6a1dd4..f786f59d9b 100644 --- a/lib/brakeman/checks/check_sql.rb +++ b/lib/brakeman/checks/check_sql.rb @@ -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 @@ -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 diff --git a/test/apps/rails6/app/models/group.rb b/test/apps/rails6/app/models/group.rb index 4245ed17a9..79a75a0212 100644 --- a/test/apps/rails6/app/models/group.rb +++ b/test/apps/rails6/app/models/group.rb @@ -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 diff --git a/test/tests/rails6.rb b/test/tests/rails6.rb index cdc3d5e734..b7724d2061 100644 --- a/test/tests/rails6.rb +++ b/test/tests/rails6.rb @@ -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,