diff --git a/lib/brakeman/checks/check_sql.rb b/lib/brakeman/checks/check_sql.rb index 29a7da4a967fd35be67738554db69960c091401d..86a3a35ee5557070ec13623e592bdda213b21e00 100644 --- a/lib/brakeman/checks/check_sql.rb +++ b/lib/brakeman/checks/check_sql.rb @@ -179,13 +179,13 @@ class Brakeman::CheckSQL < Brakeman::BaseCheck check_order_arguments call.arglist when :joins check_joins_arguments call.first_arg - when :from, :select + when :from unsafe_sql? call.first_arg when :lock check_lock_arguments call.first_arg when :pluck unsafe_sql? call.first_arg - when :update_all + when :update_all, :select check_update_all_arguments call.args when *@connection_calls check_by_sql_arguments call.first_arg diff --git a/test/apps/rails4/app/controllers/friendly_controller.rb b/test/apps/rails4/app/controllers/friendly_controller.rb index 4f97c7955adc8a72ee848a550d3c333730b189b6..6514322470e9d661306c6e53b822286eb0e25809 100644 --- a/test/apps/rails4/app/controllers/friendly_controller.rb +++ b/test/apps/rails4/app/controllers/friendly_controller.rb @@ -63,4 +63,8 @@ class FriendlyController redirect_to params.merge(:host => params[:host]) # Should warn end end + + def select_some_stuff + User.select(:name, params[:x]) + end end diff --git a/test/tests/rails4.rb b/test/tests/rails4.rb index c3187ecbfc4ad444fc9a1337b12014a6f859eef5..b103cfa452f0e5030a6a5806f50afe13e1ff7082 100644 --- a/test/tests/rails4.rb +++ b/test/tests/rails4.rb @@ -15,7 +15,7 @@ class Rails4Tests < Test::Unit::TestCase :controller => 0, :model => 1, :template => 2, - :generic => 20 + :generic => 21 } end @@ -236,6 +236,18 @@ class Rails4Tests < Test::Unit::TestCase :user_input => s(:call, s(:self), :type) end + def test_sql_injection_in_select_args + assert_warning :type => :warning, + :warning_code => 0, + :fingerprint => "bd8c539a645aa417d538cbe7b658cc1c9743f61d1e90c948afacc7e023b30a62", + :warning_type => "SQL Injection", + :line => 64, + :message => /^Possible\ SQL\ injection/, + :confidence => 0, + :relative_path => "app/controllers/friendly_controller.rb", + :user_input => s(:call, s(:params), :[], s(:lit, :x)) + end + def test_i18n_xss_CVE_2013_4491_workaround assert_no_warning :type => :warning, :warning_code => 63,