Skip to content

Commit

Permalink
Ignore I18N.locale in SQL
Browse files Browse the repository at this point in the history
Fixes #1597
  • Loading branch information
presidentbeef committed Dec 16, 2021
1 parent 743eebd commit e3981c1
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 1 deletion.
10 changes: 10 additions & 0 deletions lib/brakeman/checks/base_check.rb
Original file line number Diff line number Diff line change
Expand Up @@ -513,4 +513,14 @@ def string_building? exp
string_building? exp.target or
string_building? exp.first_arg
end

I18N_CLASS = s(:const, :I18n)

def locale_call? exp
return unless call? exp

(exp.target == I18N_CLASS and
exp.method == :locale) or
locale_call? exp.target
end
end
3 changes: 2 additions & 1 deletion lib/brakeman/checks/check_sql.rb
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,8 @@ def ignore_call? exp
arel? exp or
exp.method.to_s.end_with? "_id" or
number_target? exp or
date_target? exp
date_target? exp or
locale_call? exp
end

QUOTE_METHODS = [:quote, :quote_column_name, :quoted_date, :quote_string, :quote_table_name]
Expand Down
4 changes: 4 additions & 0 deletions test/apps/rails6/app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,8 @@ def check_enum
end

enum "stuff_#{stuff}": [:things]

def locale
User.where("lower(slug_#{I18n.locale.to_s.split("-").first}) = :country_id", country_id: params[:new_country_id]).first
end
end
13 changes: 13 additions & 0 deletions test/tests/rails6.rb
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,19 @@ def test_sql_injection_enum
:user_input => s(:call, s(:call, s(:const, :User), :states), :[], s(:str, "pending"))
end

def test_sql_injection_locale
assert_no_warning :type => :warning,
:warning_code => 0,
:fingerprint => "843143d5a9dcfa097c66d80a6a72ba151c0332f1ea8c8bc852e418d4f0e2cb7b",
:warning_type => "SQL Injection",
:line => 37,
:message => /^Possible\ SQL\ injection/,
:confidence => 1,
:relative_path => "app/models/user.rb",
:code => s(:call, s(:const, :User), :where, s(:dstr, "lower(slug_", s(:evstr, s(:call, s(:call, s(:call, s(:call, s(:const, :I18n), :locale), :to_s), :split, s(:str, "-")), :first)), s(:str, ") = :country_id")), s(:hash, s(:lit, :country_id), s(:call, s(:params), :[], s(:lit, :new_country_id)))),
:user_input => s(:call, s(:call, s(:call, s(:call, s(:const, :I18n), :locale), :to_s), :split, s(:str, "-")), :first)
end

def test_dangerous_send_enum
assert_no_warning :type => :warning,
:warning_code => 23,
Expand Down

0 comments on commit e3981c1

Please sign in to comment.