diff --git a/lib/brakeman/checks/check_forgery_setting.rb b/lib/brakeman/checks/check_forgery_setting.rb index e2f9651a546d28de2fc1f55642a58742576b2afb..024cb893efb08d7e08c9b0206dee6b7f1063708d 100644 --- a/lib/brakeman/checks/check_forgery_setting.rb +++ b/lib/brakeman/checks/check_forgery_setting.rb @@ -52,6 +52,24 @@ class Brakeman::CheckForgerySetting < Brakeman::BaseCheck :confidence => CONFIDENCE[:high], :gem_info => gemfile_or_environment, :link_path => "https://groups.google.com/d/topic/rubyonrails-security/LZWjzCPgNmU/discussion" + elsif version_between? "4.0.0", "100.0.0" and forgery_opts = app_controller[:options][:protect_from_forgery] + + unless forgery_opts.is_a?(Array) and sexp?(forgery_opts.first) and + access_arg = hash_access(forgery_opts.first.first_arg, :with) and access_arg.value == :exception + + args = { + :controller => :ApplicationController, + :warning_type => "Cross-Site Request Forgery", + :warning_code => :csrf_not_protected_by_raising_exception, + :message => "protect_from_forgery should be configured with 'with: :exception'", + :confidence => CONFIDENCE[:med], + :file => app_controller[:files].first + } + + args.merge!(:code => forgery_opts.first) if forgery_opts.is_a?(Array) + + warn args + end end end end diff --git a/lib/brakeman/warning_codes.rb b/lib/brakeman/warning_codes.rb index b86c5170c7921f04d97d2228a27e8e3a3edbb923..28cfb4d51eae6835fb1d0870bfc4329a12d268a1 100644 --- a/lib/brakeman/warning_codes.rb +++ b/lib/brakeman/warning_codes.rb @@ -87,6 +87,7 @@ module Brakeman::WarningCodes :CVE_2011_2932 => 83, :cross_site_scripting_inline => 84, :CVE_2014_7829 => 85, + :csrf_not_protected_by_raising_exception => 86, } def self.code name diff --git a/test/apps/rails4_with_engines/app/controllers/application_controller.rb b/test/apps/rails4_with_engines/app/controllers/application_controller.rb index d83690e1b9a6bdd8a08754b38231799acefcb2ab..95aaaf1a5c500b4b47a38eebdcefa84255b46af9 100644 --- a/test/apps/rails4_with_engines/app/controllers/application_controller.rb +++ b/test/apps/rails4_with_engines/app/controllers/application_controller.rb @@ -1,5 +1,5 @@ class ApplicationController < ActionController::Base # Prevent CSRF attacks by raising an exception. # For APIs, you may want to use :null_session instead. - protect_from_forgery with: :exception + protect_from_forgery end diff --git a/test/tests/rails4_with_engines.rb b/test/tests/rails4_with_engines.rb index bc4bbd8f3f2d7b39625db3e7cbef0e8481190025..cfd54d7f17f12f3c59afb6e1c88cae67e15c186b 100644 --- a/test/tests/rails4_with_engines.rb +++ b/test/tests/rails4_with_engines.rb @@ -8,7 +8,7 @@ class Rails4WithEnginesTests < Test::Unit::TestCase def expected @expected ||= { - :controller => 0, + :controller => 1, :model => 5, :template => 11, :generic => 8 } @@ -276,4 +276,13 @@ class Rails4WithEnginesTests < Test::Unit::TestCase :relative_path => "engines/user_removal/app/models/user.rb" end + def test_csrf_without_exception + assert_warning :type => :controller, + :warning_code => 86, + :fingerprint => "4d109bd02e4ccb3ea4c51485c947be435ee006a61af7d2cd37d1b358c7469189", + :warning_type => "Cross-Site Request Forgery", + :message => "protect_from_forgery should be configured with 'with: :exception'", + :confidence => 1, + :relative_path => "app/controllers/application_controller.rb" + end end