From b68bddd37af922eddbbdf21a7152da049fe442bf Mon Sep 17 00:00:00 2001 From: Justin Collins Date: Wed, 12 May 2021 14:05:52 -0700 Subject: [PATCH] Check for evaluation even if it's a call target Fixes #1590 --- lib/brakeman/checks/check_evaluation.rb | 2 +- .../rails6/app/controllers/accounts_controller.rb | 4 ++++ test/tests/github_output.rb | 2 +- test/tests/rails6.rb | 15 ++++++++++++++- 4 files changed, 20 insertions(+), 3 deletions(-) diff --git a/lib/brakeman/checks/check_evaluation.rb b/lib/brakeman/checks/check_evaluation.rb index 2a36c31aa8..2bc2dfb341 100644 --- a/lib/brakeman/checks/check_evaluation.rb +++ b/lib/brakeman/checks/check_evaluation.rb @@ -10,7 +10,7 @@ class Brakeman::CheckEvaluation < Brakeman::BaseCheck #Process calls def run_check Brakeman.debug "Finding eval-like calls" - calls = tracker.find_call :method => [:eval, :instance_eval, :class_eval, :module_eval] + calls = tracker.find_call methods: [:eval, :instance_eval, :class_eval, :module_eval], nested: true Brakeman.debug "Processing eval-like calls" calls.each do |call| diff --git a/test/apps/rails6/app/controllers/accounts_controller.rb b/test/apps/rails6/app/controllers/accounts_controller.rb index 151ddf1d51..c4f39746ed 100644 --- a/test/apps/rails6/app/controllers/accounts_controller.rb +++ b/test/apps/rails6/app/controllers/accounts_controller.rb @@ -22,4 +22,8 @@ def auth_something # Do something benign end end + + def eval_something + eval(params[:x]).to_s + end end diff --git a/test/tests/github_output.rb b/test/tests/github_output.rb index 635fa11920..8a590f1630 100644 --- a/test/tests/github_output.rb +++ b/test/tests/github_output.rb @@ -6,7 +6,7 @@ def setup end def test_report_format - assert_equal 33, @@report.lines.count + assert_equal 34, @@report.lines.count @@report.lines.each do |line| assert line.start_with?('::'), 'Every line must start with `::`' assert_equal 2, line.scan('::').count, 'Every line must have exactly 2 `::`' diff --git a/test/tests/rails6.rb b/test/tests/rails6.rb index 0b340f2cf7..6759a2754b 100644 --- a/test/tests/rails6.rb +++ b/test/tests/rails6.rb @@ -13,7 +13,7 @@ def expected :controller => 0, :model => 0, :template => 4, - :generic => 28 + :generic => 29 } end @@ -587,4 +587,17 @@ def test_skip_dev_environment :code => s(:call, nil, :eval, s(:call, s(:params), :[], s(:lit, :x))), :user_input => s(:call, s(:params), :[], s(:lit, :x)) end + + def test_dangerous_eval_as_method_target + assert_warning :type => :warning, + :warning_code => 13, + :fingerprint => "3c4b94f3fc4ff4cfb005299349eb4f9a89832f35fc33ed9edc8481b98a047edb", + :warning_type => "Dangerous Eval", + :line => 27, + :message => /^User\ input\ in\ eval/, + :confidence => 0, + :relative_path => "app/controllers/accounts_controller.rb", + :code => s(:call, nil, :eval, s(:call, s(:params), :[], s(:lit, :x))), + :user_input => s(:call, s(:params), :[], s(:lit, :x)) + end end