Merge pull request #766 from presidentbeef/fix_call_index_issues

Fix call index issues

lib/brakeman/checks/check_mass_assignment.rb

@@ -155,7 +155,7 @@ def literal? exp
   # Look for and warn about uses of Parameters#permit! for mass assignment
   def check_permit!
     tracker.find_call(:method => :permit!).each do |result|
-      if params? result[:target]
+      if params? result[:call].target
         warn_on_permit! result
       end
     end

lib/brakeman/processors/lib/find_all_calls.rb

@@ -126,7 +126,7 @@ def process_attrasgn exp
 
   #Gets the target of a call as a Symbol
   #if possible
-  def get_target exp
+  def get_target exp, include_calls = false
     if sexp? exp
       case exp.node_type
       when :ivar, :lvar, :const, :lit
@@ -137,6 +137,23 @@ def get_target exp
         class_name exp
       when :self
         @current_class || @current_module || nil
+      when :params, :session, :cookies
+        exp.node_type
+      when :call
+        if include_calls
+          if exp.target.nil?
+            exp.method
+          else
+            t = get_target(exp.target, :include_calls)
+            if t.is_a? Symbol
+              :"#{t}.#{exp.method}"
+            else
+              exp
+            end
+          end
+        else
+          exp
+        end
       else
         exp
       end
@@ -187,6 +204,8 @@ def create_call_hash exp
       @in_target = true
       process target
       @in_target = already_in_target
+
+      target = get_target(target, :include_calls)
     end
 
     method = exp.method

test/tests/call_index.rb

@@ -1,19 +1,32 @@
+require 'brakeman/processors/lib/find_all_calls'
+
 class CallIndexTests < Test::Unit::TestCase
   def setup
     @calls = [
-      {:method => :hello, :target => :world, :call => {} },
-      {:method => :goodbye, :target => :world, :call => {} },
-      {:method => :foo, :target => :world, :call => {} },
-      {:method => :foo, :target => :the_bar, :call => {} },
-      {:method => :foo, :target => :the_baz, :call => {} },
-      {:method => :do_it, :target => nil, :call => {} },
-      {:method => :do_it_now, :target => nil, :call => {} },
+      {:method => :hello, :target => :world, :call => {}, :nested => false },
+      {:method => :goodbye, :target => :world, :call => {}, :nested => false  },
+      {:method => :foo, :target => :world, :call => {}, :nested => false  },
+      {:method => :foo, :target => :the_bar, :call => {}, :nested => false  },
+      {:method => :foo, :target => :the_baz, :call => {}, :nested => false  },
+      {:method => :do_it, :target => nil, :call => {}, :nested => false  },
+      {:method => :do_it_now, :target => nil, :call => {}, :nested => false  },
     ]
+
+    src = Brakeman::AliasProcessor.new.process RubyParser.new.parse <<-RUBY
+      def x
+        x.y.z(1)
+        params[:x].y.z(2)
+      end
+    RUBY
+    all_calls = Brakeman::FindAllCalls.new(Object.new)
+    all_calls.process(src)
+    @calls += all_calls.calls
+
     @call_index = Brakeman::CallIndex.new(@calls)
   end
 
   def assert_found num, opts
-    assert @call_index.find_calls(opts).length
+    assert_equal num, @call_index.find_calls(opts).length
   end
 
   def test_find_by_method_regex
@@ -55,4 +68,14 @@ def test_find_by_no_target_and_method
   def test_find_by_no_target_and_methods
     assert_found 2, :target => nil, :method => [:do_it, :do_it_now]
   end
+
+  def test_find_by_target_and_method_in_chain
+    assert_found 0, :target => :x, :method => :z
+    assert_found 1, :target => :x, :method => :z, :chained => true
+  end
+
+  def test_find_params_and_method_in_chain
+    assert_found 0, :target => :params, :method => :z
+    assert_found 1, :target => :params, :method => :z, :chained => true
+  end
 end