Skip to content

Commit

Permalink
Treat (Hash|Array)#fetch like (Hash|Array)#[]
Browse files Browse the repository at this point in the history
One of the issues from #1571
  • Loading branch information
presidentbeef committed May 3, 2021
1 parent 2ece421 commit eabb04b
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 0 deletions.
9 changes: 9 additions & 0 deletions lib/brakeman/processors/alias_processor.rb
Expand Up @@ -227,6 +227,15 @@ def process_call exp
elsif hash? target
exp = process_hash_access(target, first_arg, exp)
end
when :fetch
if array? target
# Not dealing with default value
# so just pass in first argument, but process_array_access expects
# an array of arguments.
exp = process_array_access(target, [first_arg], exp)
elsif hash? target
exp = process_hash_access(target, first_arg, exp)
end
when :merge!, :update
if hash? target and hash? first_arg
target = process_hash_merge! target, first_arg
Expand Down
6 changes: 6 additions & 0 deletions test/apps/rails6/app/models/group.rb
Expand Up @@ -7,4 +7,10 @@ def date_in_sql
date = 30.days.ago
Arel.sql("created_at > '#{date}'")
end

def fetch_constant_hash_value(role_name)
roles = { admin: 1, moderator: 2 }.freeze
role = roles.fetch(role_name)
Arel.sql("role = '#{role}'")
end
end
13 changes: 13 additions & 0 deletions test/tests/alias_processor.rb
Expand Up @@ -123,6 +123,12 @@ def test_array_negative_index
RUBY
end

def test_array_fetch
assert_alias '3', <<-RUBY
x = [1, 2, 3]
x.fetch(2)
RUBY
end

def test_array_append
assert_alias '[1, 2, 3]', <<-RUBY
Expand Down Expand Up @@ -208,6 +214,13 @@ def test_hash_new_index
RUBY
end

def test_hash_fetch
assert_alias '1', <<-RUBY
x = { a: 0, b: 1, c: 3 }
x.fetch(:b)
RUBY
end

def test_hash_update
assert_alias "2", <<-RUBY
@foo = {
Expand Down
13 changes: 13 additions & 0 deletions test/tests/rails6.rb
Expand Up @@ -134,6 +134,19 @@ def test_sql_injection_date_integer_target_false_positive
:user_input => s(:call, s(:call, s(:lit, 30), :days), :ago)
end

def test_sql_injection_hash_fetch_all_literals
assert_no_warning :type => :warning,
:warning_code => 0,
:fingerprint => "1b9a04c9fdc4b8c7f215387fb726dc542c2d35dde2f29b48a76248443524a5fa",
:warning_type => "SQL Injection",
:line => 14,
:message => /^Possible\ SQL\ injection/,
:confidence => 1,
:relative_path => "app/models/group.rb",
:code => s(:call, s(:const, :Arel), :sql, s(:dstr, "role = '", s(:evstr, s(:call, s(:hash, s(:lit, :admin), s(:lit, 1), s(:lit, :moderator), s(:lit, 2)), :fetch, s(:lvar, :role_name))), s(:str, "'"))),
:user_input => s(:call, s(:hash, s(:lit, :admin), s(:lit, 1), s(:lit, :moderator), s(:lit, 2)), :fetch, s(:lvar, :role_name))
end

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

0 comments on commit eabb04b

Please sign in to comment.