Merge pull request #966 from presidentbeef/handle_concern_included_in…

…_controllers

Handle concern included in controllers

lib/brakeman/app_tree.rb

@@ -89,15 +89,15 @@ def path_exists?(path)
     end
 
     def initializer_paths
-      @initializer_paths ||= find_paths("config/initializers")
+      @initializer_paths ||= prioritize_concerns(find_paths("config/initializers"))
     end
 
     def controller_paths
-      @controller_paths ||= find_paths("app/**/controllers")
+      @controller_paths ||= prioritize_concerns(find_paths("app/**/controllers"))
     end
 
     def model_paths
-      @model_paths ||= find_paths("app/**/models")
+      @model_paths ||= prioritize_concerns(find_paths("app/**/models"))
     end
 
     def template_paths
@@ -177,5 +177,9 @@ def root_search_pattern
       rel_engines = (rel + [""]).join("/,")
       @root_search_patrern = "{#{roots}}/{#{rel_engines}}"
     end
+
+    def prioritize_concerns paths
+      paths.partition { |path| path.include? "concerns" }.flatten
+    end
   end
 end

lib/brakeman/processors/controller_processor.rb

@@ -61,6 +61,16 @@ def process_module exp, parent = nil
     handle_module exp, Brakeman::Controller, parent
   end
 
+  def process_concern concern_name
+    return unless @current_class
+
+    if mod = @tracker.find_class(concern_name)
+      if mod.options[:included]
+        process mod.options[:included].deep_clone
+      end
+    end
+  end
+
   #Look for specific calls inside the controller
   def process_call exp
     return exp if process_call_defn? exp
@@ -89,7 +99,11 @@ def process_call exp
       else
         case method
         when :include
-          @current_class.add_include class_name(first_arg) if @current_class
+          if @current_class
+            concern = class_name(first_arg)
+            @current_class.add_include concern
+            process_concern concern
+          end
         when :before_filter, :append_before_filter, :before_action, :append_before_action
           if node_type? exp.first_arg, :iter
             add_lambda_filter exp

lib/brakeman/processors/library_processor.rb

@@ -51,4 +51,16 @@ def process_call exp
       process_default exp
     end
   end
+
+  def process_iter exp
+    res = process_default exp
+
+    if node_type? res, :iter and call? exp.block_call # sometimes this changes after processing
+      if exp.block_call.method == :included
+        (@current_module || @current_class).options[:included] = res.block
+      end
+    end
+
+    res
+  end
 end

lib/brakeman/tracker.rb

@@ -198,6 +198,16 @@ def constant_lookup name
     @constants.get_literal name unless @options[:disable_constant_tracking]
   end
 
+  def find_class name
+    [@controllers, @models, @libs].each do |collection|
+      if c = collection[name]
+        return c
+      end
+    end
+
+    nil
+  end
+
   def index_call_sites
     finder = Brakeman::FindAllCalls.new self
 

test/apps/rails5/app/controllers/concerns/forgery_protection.rb

@@ -0,0 +1,7 @@
+module ForgeryProtection
+  extend ActiveSupport::Concern
+
+  included do
+    protect_from_forgery with: :exception
+  end
+end

test/apps/rails5/app/controllers/mixed_controller.rb

@@ -0,0 +1,4 @@
+class BaseController < ActionController::Base
+  # No protect_from_forgery call, but one mixed in
+  include ForgeryProtection
+end

test/tests/rails5.rb

@@ -410,4 +410,12 @@ def test_link_to_href_safe_interpolation
       :code => s(:call, nil, :link_to, s(:str, "Email!"), s(:dstr, "mailto:", s(:evstr, s(:call, s(:params), :[], s(:lit, :x))))),
       :user_input => s(:call, s(:params), :[], s(:lit, :x))
   end
+
+  def test_mixed_in_csrf_protection
+    assert_no_warning :type => :controller,
+      :warning_type => "Cross-Site Request Forgery",
+      :line => 1,
+      :message => /^'protect_from_forgery'\ should\ be\ called\ /,
+      :relative_path => "app/controllers/mixed_controller.rb"
+  end
 end