diff --git a/lib/brakeman/checks/check_sql.rb b/lib/brakeman/checks/check_sql.rb index 8b67e4694a..fd77ea95a6 100644 --- a/lib/brakeman/checks/check_sql.rb +++ b/lib/brakeman/checks/check_sql.rb @@ -22,7 +22,19 @@ def run_check :find_by_sql, :maximum, :minimum, :pluck, :sum, :update_all] @sql_targets.concat [:from, :group, :having, :joins, :lock, :order, :reorder, :where] if tracker.options[:rails3] @sql_targets.concat [:find_by, :find_by!, :find_or_create_by, :find_or_create_by!, :find_or_initialize_by, :not] if tracker.options[:rails4] - @sql_targets << :delete_by << :destroy_by if tracker.options[:rails6] + + if tracker.options[:rails6] + @sql_targets.concat [:delete_by, :destroy_by, :rewhere, :reselect] + + @sql_targets.delete :delete_all + @sql_targets.delete :destroy_all + @sql_targets.delete :order + @sql_targets.delete :reorder + end + + if version_between?("6.1.0", "9.9.9") + @sql_targets.delete :pluck + end if version_between?("2.0.0", "3.9.9") or tracker.config.rails_version.nil? @sql_targets << :first << :last << :all @@ -185,7 +197,7 @@ def process_result result else check_find_arguments call.last_arg end - when :where, :having, :find_by, :find_by!, :find_or_create_by, :find_or_create_by!, :find_or_initialize_by,:not, :delete_by, :destroy_by + when :where, :rewhere, :having, :find_by, :find_by!, :find_or_create_by, :find_or_create_by!, :find_or_initialize_by,:not, :delete_by, :destroy_by check_query_arguments call.arglist when :order, :group, :reorder check_order_arguments call.arglist @@ -199,7 +211,7 @@ def process_result result unsafe_sql? call.first_arg when :sql unsafe_sql? call.first_arg - when :update_all, :select + when :update_all, :select, :reselect check_update_all_arguments call.args when *@connection_calls check_by_sql_arguments call.first_arg diff --git a/test/apps/rails6/app/controllers/groups_controller.rb b/test/apps/rails6/app/controllers/groups_controller.rb index 3a1babc320..4b43e9eb39 100644 --- a/test/apps/rails6/app/controllers/groups_controller.rb +++ b/test/apps/rails6/app/controllers/groups_controller.rb @@ -61,4 +61,9 @@ def scope_with_custom_sanitization def sanitize_s(input) input end + + def test_rails6_sqli + User.select("stuff").reselect(params[:columns]) + User.where("x = 1").rewhere("x = #{params[:x]}") + end end diff --git a/test/tests/github_output.rb b/test/tests/github_output.rb index c57bf8d34b..003c653002 100644 --- a/test/tests/github_output.rb +++ b/test/tests/github_output.rb @@ -6,7 +6,7 @@ def setup end def test_report_format - assert_equal 35, @@report.lines.count + assert_equal 37, @@report.lines.count, "Did you add vulnerabilities to the Rails 6 app? Update this test please!" @@report.lines.each do |line| assert line.start_with?('::'), 'Every line must start with `::`' assert_equal 2, line.scan('::').count, 'Every line must have exactly 2 `::`' diff --git a/test/tests/rails6.rb b/test/tests/rails6.rb index bff45eddda..08e5e260d8 100644 --- a/test/tests/rails6.rb +++ b/test/tests/rails6.rb @@ -13,7 +13,7 @@ def expected :controller => 0, :model => 0, :template => 4, - :generic => 29 + :generic => 31 } end @@ -186,6 +186,32 @@ def test_sql_injection_with_date :user_input => s(:call, s(:call, s(:const, :Date), :today), :-, s(:lit, 1)) end + def test_sql_injection_rewhere + assert_warning :type => :warning, + :warning_code => 0, + :fingerprint => "7f5154ba5124c5ae26ec23f364239311df959acb9b2e4d09f4867c2fbd954dd6", + :warning_type => "SQL Injection", + :line => 67, + :message => /^Possible\ SQL\ injection/, + :confidence => 0, + :relative_path => "app/controllers/groups_controller.rb", + :code => s(:call, s(:call, s(:const, :User), :where, s(:str, "x = 1")), :rewhere, s(:dstr, "x = ", s(:evstr, s(:call, s(:params), :[], s(:lit, :x))))), + :user_input => s(:call, s(:params), :[], s(:lit, :x)) + end + + def test_sql_injection_reselect + assert_warning :type => :warning, + :warning_code => 0, + :fingerprint => "e4fdd9614cff8e8f8a70cd983c55d36acd6da219048faf1530de9dc43d58aa71", + :warning_type => "SQL Injection", + :line => 66, + :message => /^Possible\ SQL\ injection/, + :confidence => 0, + :relative_path => "app/controllers/groups_controller.rb", + :code => s(:call, s(:call, s(:const, :User), :select, s(:str, "stuff")), :reselect, s(:call, s(:params), :[], s(:lit, :columns))), + :user_input => s(:call, s(:params), :[], s(:lit, :columns)) + end + def test_cross_site_scripting_sanity assert_warning :type => :template, :warning_code => 2,