From 12ed187aaa85ab560c927bc8585f75e3a6efefbf Mon Sep 17 00:00:00 2001
From: Neil Matatall <neil.matatall@gmail.com>
Date: Mon, 13 Apr 2015 18:12:14 -0700
Subject: [PATCH] Warn unless a rails 4 app protect[s]_from_forgery with
 :exception

---
 lib/brakeman/checks/check_forgery_setting.rb          | 11 +++++++++++
 lib/brakeman/warning_codes.rb                         |  1 +
 .../app/controllers/application_controller.rb         |  2 +-
 test/tests/rails4_with_engines.rb                     | 11 ++++++++++-
 4 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/lib/brakeman/checks/check_forgery_setting.rb b/lib/brakeman/checks/check_forgery_setting.rb
index e2f9651a..0952d546 100644
--- a/lib/brakeman/checks/check_forgery_setting.rb
+++ b/lib/brakeman/checks/check_forgery_setting.rb
@@ -52,6 +52,17 @@ 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 hash_access(forgery_opts.first.first_arg, :with).value == :exception
+        warn :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,
+          :link_path => "blog post link?"
+      end
     end
   end
 end
diff --git a/lib/brakeman/warning_codes.rb b/lib/brakeman/warning_codes.rb
index b86c5170..28cfb4d5 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 d83690e1..95aaaf1a 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 bc4bbd8f..cfd54d7f 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
-- 
GitLab