From 44283ebc1e4c3dfbc92beb69eb0603087d78c683 Mon Sep 17 00:00:00 2001 From: Justin Collins Date: Thu, 25 Apr 2013 14:13:21 -0700 Subject: [PATCH] Use hash for call locations instead of array --- lib/brakeman/call_index.rb | 8 +-- lib/brakeman/checks/base_check.rb | 4 +- lib/brakeman/checks/check_execute.rb | 14 +--- lib/brakeman/checks/check_select_tag.rb | 2 +- .../checks/check_select_vulnerability.rb | 4 +- lib/brakeman/checks/check_sql.rb | 9 +-- lib/brakeman/processors/lib/find_all_calls.rb | 71 +++++++++---------- lib/brakeman/warning.rb | 13 ++-- 8 files changed, 59 insertions(+), 66 deletions(-) diff --git a/lib/brakeman/call_index.rb b/lib/brakeman/call_index.rb index d9798c8d..b29f3559 100644 --- a/lib/brakeman/call_index.rb +++ b/lib/brakeman/call_index.rb @@ -82,13 +82,13 @@ class Brakeman::CallIndex def remove_indexes_by_class classes @calls_by_method.each do |name, calls| calls.delete_if do |call| - call[:location][0] == :class and classes.include? call[:location][1] + call[:location][:type] == :class and classes.include? call[:location][:class] end end @calls_by_target.each do |name, calls| calls.delete_if do |call| - call[:location][0] == :class and classes.include? call[:location][1] + call[:location][:type] == :class and classes.include? call[:location][:class] end end end @@ -206,8 +206,8 @@ class Brakeman::CallIndex end def from_template call, template_name - return false unless call[:location][0] == :template + return false unless call[:location][:type] == :template return true if template_name.nil? - call[:location][1] == template_name + call[:location][:template] == template_name end end diff --git a/lib/brakeman/checks/base_check.rb b/lib/brakeman/checks/base_check.rb index 2f7723a1..4a808667 100644 --- a/lib/brakeman/checks/base_check.rb +++ b/lib/brakeman/checks/base_check.rb @@ -31,7 +31,7 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor #Add result to result list, which is used to check for duplicates def add_result result, location = nil - location ||= (@current_template && @current_template[:name]) || @current_class || @current_module || @current_set || result[:location][1] + location ||= (@current_template && @current_template[:name]) || @current_class || @current_module || @current_set || result[:location][:class] || result[:location][:template] location = location[:name] if location.is_a? Hash location = location.to_sym @@ -244,7 +244,7 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor raise ArgumentError end - location ||= (@current_template && @current_template[:name]) || @current_class || @current_module || @current_set || result[:location][1] + location ||= (@current_template && @current_template[:name]) || @current_class || @current_module || @current_set || result[:location][:class] || result[:location][:template] location = location[:name] if location.is_a? Hash location = location.to_sym diff --git a/lib/brakeman/checks/check_execute.rb b/lib/brakeman/checks/check_execute.rb index 2e6c8681..e5d28480 100644 --- a/lib/brakeman/checks/check_execute.rb +++ b/lib/brakeman/checks/check_execute.rb @@ -84,20 +84,12 @@ class Brakeman::CheckExecute < Brakeman::BaseCheck user_input = nil end - warning = { :warning_type => "Command Injection", + warn :result => result, + :warning_type => "Command Injection", :warning_code => :command_injection, :message => "Possible command injection", :code => exp, :user_input => user_input, - :confidence => confidence } - - if result[:location][0] == :template - warning[:template] = result[:location][1] - else - warning[:class] = result[:location][1] - warning[:method] = result[:location][2] - end - - warn warning + :confidence => confidence end end diff --git a/lib/brakeman/checks/check_select_tag.rb b/lib/brakeman/checks/check_select_tag.rb index 2f7cec1c..96c1d4f4 100644 --- a/lib/brakeman/checks/check_select_tag.rb +++ b/lib/brakeman/checks/check_select_tag.rb @@ -24,7 +24,7 @@ class Brakeman::CheckSelectTag < Brakeman::BaseCheck @message = "Upgrade to Rails #{suggested_version}, #{tracker.config[:rails_version]} select_tag is vulnerable (CVE-2012-3463)" calls = tracker.find_call(:target => nil, :method => :select_tag).select do |result| - result[:location][0] == :template + result[:location][:type] == :template end calls.each do |result| diff --git a/lib/brakeman/checks/check_select_vulnerability.rb b/lib/brakeman/checks/check_select_vulnerability.rb index 15facdf1..ae26d3f5 100644 --- a/lib/brakeman/checks/check_select_vulnerability.rb +++ b/lib/brakeman/checks/check_select_vulnerability.rb @@ -24,7 +24,7 @@ class Brakeman::CheckSelectVulnerability < Brakeman::BaseCheck @message = "Upgrade to Rails #{suggested_version}, #{tracker.config[:rails_version]} select() helper is vulnerable" calls = tracker.find_call(:target => nil, :method => :select).select do |result| - result[:location][0] == :template + result[:location][:type] == :template end calls.each do |result| @@ -47,7 +47,7 @@ class Brakeman::CheckSelectVulnerability < Brakeman::BaseCheck confidence = CONFIDENCE[:low] end - warn :template => result[:location][1], + warn :template => result[:location][:template], :warning_type => "Cross Site Scripting", :warning_code => :select_options_vuln, :result => result, diff --git a/lib/brakeman/checks/check_sql.rb b/lib/brakeman/checks/check_sql.rb index 6e828bb4..7b3a11b0 100644 --- a/lib/brakeman/checks/check_sql.rb +++ b/lib/brakeman/checks/check_sql.rb @@ -68,7 +68,7 @@ class Brakeman::CheckSQL < Brakeman::BaseCheck if model[:options][:named_scope] model[:options][:named_scope].each do |args| call = make_call(nil, :named_scope, args).line(args.line) - scope_calls << { :call => call, :location => [:class, name ], :method => :named_scope } + scope_calls << { :call => call, :location => { :type => :class, :class => name }, :method => :named_scope } end end end @@ -84,10 +84,10 @@ class Brakeman::CheckSQL < Brakeman::BaseCheck process_scope_with_block name, args elsif second_arg.node_type == :call call = second_arg - scope_calls << { :call => call, :location => [:class, name ], :method => call.method } + scope_calls << { :call => call, :location => { :type => :class, :class => name }, :method => call.method } else call = make_call(nil, :scope, args).line(args.line) - scope_calls << { :call => call, :location => [:class, name ], :method => :scope } + scope_calls << { :call => call, :location => { :type => :class, :class => name }, :method => :scope } end end end @@ -174,7 +174,8 @@ class Brakeman::CheckSQL < Brakeman::BaseCheck end end elsif block.node_type == :call - process_result :target => block.target, :method => block.method, :call => block, :location => [:class, model_name, scope_name] + process_result :target => block.target, :method => block.method, :call => block, + :location => { :type => :class, :class => model_name, :method => scope_name } end end diff --git a/lib/brakeman/processors/lib/find_all_calls.rb b/lib/brakeman/processors/lib/find_all_calls.rb index debbb1f5..17e0ba5b 100644 --- a/lib/brakeman/processors/lib/find_all_calls.rb +++ b/lib/brakeman/processors/lib/find_all_calls.rb @@ -49,15 +49,12 @@ class Brakeman::FindAllCalls < Brakeman::BaseProcessor method = exp.method process_call_args exp - call = { :target => target, :method => method, :call => exp, :nested => @in_target, :chain => get_chain(exp) } - - if @current_template - call[:location] = [:template, @current_template] - else - call[:location] = [:class, @current_class, @current_method] - end - - @calls << call + @calls << { :target => target, + :method => method, + :call => exp, + :nested => @in_target, + :chain => get_chain(exp), + :location => make_location } exp end @@ -67,15 +64,11 @@ class Brakeman::FindAllCalls < Brakeman::BaseProcessor def process_render exp process exp.last if sexp? exp.last - call = { :target => nil, :method => :render, :call => exp, :nested => false } - - if @current_template - call[:location] = [:template, @current_template] - else - call[:location] = [:class, @current_class, @current_method] - end - - @calls << call + @calls << { :target => nil, + :method => :render, + :call => exp, + :nested => false, + :location => make_location } exp end @@ -85,15 +78,11 @@ class Brakeman::FindAllCalls < Brakeman::BaseProcessor def process_dxstr exp process exp.last if sexp? exp.last - call = { :target => nil, :method => :`, :call => exp, :nested => false } - - if @current_template - call[:location] = [:template, @current_template] - else - call[:location] = [:class, @current_class, @current_method] - end - - @calls << call + @calls << { :target => nil, + :method => :`, + :call => exp, + :nested => false, + :location => make_location } exp end @@ -102,15 +91,11 @@ class Brakeman::FindAllCalls < Brakeman::BaseProcessor def process_dsym exp exp.each { |arg| process arg if sexp? arg } - call = { :target => nil, :method => :literal_to_sym, :call => exp, :nested => false } - - if @current_template - call[:location] = [:template, @current_template] - else - call[:location] = [:class, @current_class, @current_method] - end - - @calls << call + @calls << { :target => nil, + :method => :literal_to_sym, + :call => exp, + :nested => false, + :location => make_location } exp end @@ -156,4 +141,18 @@ class Brakeman::FindAllCalls < Brakeman::BaseProcessor [get_target(call)] end end + + def make_location + if @current_template + { :type => :template, + :template => @current_template, + :file => @current_file } + else + { :type => :class, + :class => @current_class, + :method => @current_method, + :file => @current_file } + end + + end end diff --git a/lib/brakeman/warning.rb b/lib/brakeman/warning.rb index a801210d..02069952 100644 --- a/lib/brakeman/warning.rb +++ b/lib/brakeman/warning.rb @@ -24,13 +24,14 @@ class Brakeman::Warning result = options[:result] if result - if result[:location][0] == :template #template result - @template ||= result[:location][1] - @code ||= result[:call] + @code ||= result[:call] + @file ||= result[:file] + + if result[:location][:type] == :template #template result + @template ||= result[:location][:template] else - @class ||= result[:location][1] - @method ||= result[:location][2] - @code ||= result[:call] + @class ||= result[:location][:class] + @method ||= result[:location][:method] end end -- GitLab