Merge pull request #472 from presidentbeef/fix_chained_calls_in_single_line_scopes

Fix checking chained SQL calls in scopes

lib/brakeman/checks/check_sql.rb

@@ -65,7 +65,7 @@ class Brakeman::CheckSQL < Brakeman::BaseCheck
         call = make_call(nil, :named_scope, args).line(args.line)
         scope_calls << scope_call_hash(call, name, :named_scope)
       end
-    elsif version_between?("3.1.0", "3.9.9")
+    elsif version_between?("3.1.0", "4.9.9")
       ar_scope_calls(:scope) do |name, args|
         second_arg = args[2]
         next unless sexp? second_arg
@@ -114,8 +114,12 @@ class Brakeman::CheckSQL < Brakeman::BaseCheck
       find_calls.process_source(block, :class => model_name, :method => scope_name)
       find_calls.calls.each { |call| process_result(call) if @sql_targets.include?(call[:method]) }
     elsif block.node_type == :call
-      process_result :target => block.target, :method => block.method, :call => block,
-        :location => { :type => :class, :class => model_name, :method => scope_name }
+      while call? block
+        process_result :target => block.target, :method => block.method, :call => block,
+         :location => { :type => :class, :class => model_name, :method => scope_name }
+
+        block = block.target
+      end
     end
   end
 

test/apps/rails4/app/models/user.rb

@@ -2,4 +2,6 @@ class User < ActiveRecord::Base
   def test_sql_sanitize(x)
     self.select("#{sanitize(x)}")
   end
+
+  scope :hits_by_ip, ->(ip,col="*") { select("#{col}").order("id DESC") }
 end

test/tests/rails4.rb

@@ -15,7 +15,7 @@ class Rails4Tests < Test::Unit::TestCase
       :controller => 0,
       :model => 1,
       :template => 2,
-      :generic => 21
+      :generic => 22
     }
   end
 
@@ -270,6 +270,18 @@ class Rails4Tests < Test::Unit::TestCase
       :user_input => s(:call, nil, :sanitize, s(:lvar, :x))
   end
 
+  def test_sql_injection_chained_call_in_scope
+    assert_warning :type => :warning,
+      :warning_code => 0,
+      :fingerprint => "aa073ab210f9f4a800b5595241a6274656d37087a4f433d4b596516e1227d91b",
+      :warning_type => "SQL Injection",
+      :line => 6,
+      :message => /^Possible\ SQL\ injection/,
+      :confidence => 1,
+      :relative_path => "app/models/user.rb",
+      :user_input => s(:lvar, :col)
+  end
+
   def test_i18n_xss_CVE_2013_4491_workaround
     assert_no_warning :type => :warning,
       :warning_code => 63,