CheckSQL: Add check for Model#select

lib/brakeman/checks/check_sql.rb

@@ -20,7 +20,7 @@ class Brakeman::CheckSQL < Brakeman::BaseCheck
       :find, :find_by_sql, :first, :last, :maximum, :minimum, :sum]
 
     if tracker.options[:rails3]
-      @sql_targets.concat [:from, :group, :having, :joins, :lock, :order, :reorder, :where]
+      @sql_targets.concat [:from, :group, :having, :joins, :lock, :order, :reorder, :select, :where]
     end
 
     Brakeman.debug "Finding possible SQL calls on models"
@@ -159,7 +159,7 @@ class Brakeman::CheckSQL < Brakeman::BaseCheck
                         check_order_arguments args
                       when :joins
                         check_joins_arguments args[1]
-                      when :from
+                      when :from, :select
                         unsafe_sql? args[1]
                       when :lock
                         check_lock_arguments args[1]

test/apps/rails3.1/app/models/product.rb

@@ -142,4 +142,12 @@ class Product < ActiveRecord::Base
     Product.average(:price, :conditions => ["blah = #{params[:columns]} and x = ?", x])
     Product.sum(params[:columns])
   end
+
+  def test_select
+    #Should not warn
+    Product.select([:price, :sku])
+
+    #Should warn
+    Product.select params[:columns]
+  end
 end

test/tests/test_rails31.rb

@@ -15,7 +15,7 @@ class Rails31Tests < Test::Unit::TestCase
       :model => 0,
       :template => 2,
       :controller => 1,
-      :warning => 35 }
+      :warning => 36 }
   end
 
   def test_without_protection
@@ -351,6 +351,15 @@ class Rails31Tests < Test::Unit::TestCase
       :file => /product\.rb/
   end
 
+  def test_sql_injection_in_select
+    assert_warning :type => :warning,
+      :warning_type => "SQL Injection",
+      :line => 151,
+      :message => /^Possible\ SQL\ injection/,
+      :confidence => 0,
+      :file => /product\.rb/
+  end
+
   def test_select_vulnerability
     assert_warning :type => :template,
       :warning_type => "Cross Site Scripting",