diff --git a/lib/brakeman/call_index.rb b/lib/brakeman/call_index.rb index d9798c8dd5d7af62d0b1ce05415e38ce27635997..b29f3559da7ce3b59762a044ae0fbbaa06b43c03 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 2f7723a1547f12897c347c5dbb9670de07a16a52..4a8086672882a320c377c5b71fe08d0a06f865f9 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 2e6c86819d2e323e15ad1737ba7dcb7938c361e3..e5d2848099f54d215169bd58e8724d1c615f8ed1 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 2f7cec1cffa18aa79d785c31024f9a3b89e8f016..96c1d4f4fd58c9317439172605c3f60227951152 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 15facdf1ae24a6bfdebb1c8b35bf34480e129991..ae26d3f5bcf75a5df2d97163c043ab5a36b865a4 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 6e828bb4d5815b5e21ee5f2bd88882feaf2a9071..7b3a11b0e3389c05485bffb5de181600179f4978 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 debbb1f5217f62a6d9a08541b5f1c91772b64d29..17e0ba5be88c6774f30d198c10fef694cf7d43b0 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 a801210db826ffffbdabf3b68a72cfd6eb8d6cb2..0206995263e79325c12f3f3b7dfed4ed84f5aacf 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