Merge pull request #648 from oreoshake/check-for-csrf-protection-by-exception

Warn unless a rails 4 app protect[s]_from_forgery with :exception

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

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

test/apps/rails4_with_engines/app/controllers/application_controller.rb

 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

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