From 35a482f7f726d492382beb14d8c83955202dc1b3 Mon Sep 17 00:00:00 2001 From: Justin Collins Date: Thu, 13 Feb 2025 16:35:10 -0700 Subject: [PATCH] Refactoring CheckEvaluation --- lib/brakeman/checks/check_evaluation.rb | 60 ++++++++++++------------- 1 file changed, 28 insertions(+), 32 deletions(-) diff --git a/lib/brakeman/checks/check_evaluation.rb b/lib/brakeman/checks/check_evaluation.rb index 284a507a5..e8be6f4ad 100644 --- a/lib/brakeman/checks/check_evaluation.rb +++ b/lib/brakeman/checks/check_evaluation.rb @@ -22,49 +22,45 @@ def run_check def process_result result return unless original? result - if input = include_user_input?(result[:call].arglist) - confidence = :high - message = msg(msg_input(input), " evaluated as code") - elsif string_evaluation? result[:call].first_arg - confidence = :low - message = "Dynamic string evaluated as code" - elsif safe_literal? result[:call].first_arg - # don't warn - elsif result[:call].method == :eval - confidence = :low - message = "Dynamic code evaluation" - end + first_arg = result[:call].first_arg + + unless safe_value? first_arg + if input = include_user_input?(first_arg) + confidence = :high + message = msg(msg_input(input), " evaluated as code") + elsif string_evaluation? first_arg + confidence = :low + message = "Dynamic string evaluated as code" + elsif result[:call].method == :eval + confidence = :low + message = "Dynamic code evaluation" + end - if confidence - warn :result => result, - :warning_type => "Dangerous Eval", - :warning_code => :code_eval, - :message => message, - :user_input => input, - :confidence => confidence, - :cwe_id => [913, 95] + if confidence + warn :result => result, + :warning_type => "Dangerous Eval", + :warning_code => :code_eval, + :message => message, + :user_input => input, + :confidence => confidence, + :cwe_id => [913, 95] + end end end def string_evaluation? exp - (string_interp? exp and not all_safe_interp_values? exp) or + string_interp? exp or (call? exp and string? exp.target) end - def all_safe_interp_values? exp - exp.all? do |e| - if sexp? e - safe_interp_value? e - else - true # not an s-exp - end - end - end + def safe_value? exp + return true unless sexp? exp - def safe_interp_value? exp case exp.sexp_type + when :dstr + exp.all? { |e| safe_value? e} when :evstr - safe_interp_value? exp.value + safe_value? exp.value when :str, :lit true when :call