Merge branch 'support_strong_parameters2'

Conflicts:
	test/apps/rails3/app/controllers/other_controller.rb

lib/brakeman/checks/base_check.rb

@@ -144,7 +144,7 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor
 
   # go up the chain of parent classes to see if any have attr_accessible
   def parent_classes_protected? model
-    if model[:attr_accessible]
+    if model[:attr_accessible] or model[:includes].include? :"ActiveModel::ForbiddenAttributesProtection"
       true
     elsif parent = tracker.models[model[:parent]]
       parent_classes_protected? parent
@@ -159,21 +159,28 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor
 
     @mass_assign_disabled = false
 
-    if version_between?("3.1.0", "4.0.0") and
+    if version_between?("3.1.0", "3.9.9") and
       tracker.config[:rails] and
       tracker.config[:rails][:active_record] and
       tracker.config[:rails][:active_record][:whitelist_attributes] == Sexp.new(:true)
 
+      @mass_assign_disabled = true
+    elsif version_between?("4.0.0", "4.9.9")
+      #May need to revisit dependng on what Rails 4 actually does/has
       @mass_assign_disabled = true
     else
       matches = tracker.check_initializers(:"ActiveRecord::Base", :send)
 
       if matches.empty?
+        #Check for
+        #  class ActiveRecord::Base
+        #    attr_accessible nil
+        #  end
         matches = tracker.check_initializers([], :attr_accessible)
 
         matches.each do |result|
-          if result[1] == "ActiveRecord" and result[2] == :Base
-            arg = result[-1].first_arg
+          if result.module == "ActiveRecord" and result.result_class == :Base
+            arg = result.call.first_arg
 
             if arg.nil? or node_type? arg, :nil
               @mass_assign_disabled = true
@@ -182,9 +189,10 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor
           end
         end
       else
+        #Check for ActiveRecord::Base.send(:attr_accessible, nil)
         matches.each do |result|
-          if call? result[-1]
-            call = result[-1]
+          call = result.call
+          if call? call
             if call.first_arg == Sexp.new(:lit, :attr_accessible) and call.second_arg == Sexp.new(:nil)
               @mass_assign_disabled = true
               break
@@ -194,6 +202,23 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor
       end
     end
 
+    #There is a chance someone is using Rails 3.x and the `strong_parameters`
+    #gem and still using hack above, so this is a separate check for
+    #including ActiveModel::ForbiddenAttributesProtection in
+    #ActiveRecord::Base in an initializer.
+    if not @mass_assign_disabled and version_between?("3.1.0", "3.9.9") and tracker.config[:gems][:strong_parameters]
+      matches = tracker.check_initializers([], :include)
+
+      matches.each do |result|
+        call = result.call
+        if call? call
+          if call.first_arg == Sexp.new(:colon2, Sexp.new(:const, :ActiveModel), :ForbiddenAttributesProtection)
+            @mass_assign_disabled = true
+          end
+        end
+      end
+    end
+
     @mass_assign_disabled
   end
 

lib/brakeman/checks/check_cross_site_scripting.rb

@@ -60,7 +60,7 @@ class Brakeman::CheckCrossSiteScripting < Brakeman::BaseCheck
 
     json_escape_on = false
     initializers = tracker.check_initializers :ActiveSupport, :escape_html_entities_in_json=
-    initializers.each {|result| json_escape_on = true?(result[-1].first_arg) }
+    initializers.each {|result| json_escape_on = true?(result.call.first_arg) }
 
     if !json_escape_on or version_between? "0.0.0", "2.0.99"
       @known_dangerous << :to_json

lib/ruby_parser/bm_sexp.rb

@@ -147,13 +147,15 @@ class Sexp
   #s(:call, s(:call, nil, :x, s(:arglist)), :y, s(:arglist, s(:lit, 1)))
   #                        ^- method
   def method
-    expect :call, :attrasgn, :super, :zsuper
+    expect :call, :attrasgn, :super, :zsuper, :result
 
     case self.node_type
     when :call, :attrasgn
       self[2]
     when :super, :zsuper
       :super
+    when :result
+      self.last
     end
   end
 
@@ -492,6 +494,27 @@ class Sexp
     expect :class
     self[2]
   end
+
+  #Returns the call Sexp in a result returned from FindCall
+  def call
+    expect :result
+
+    self.last
+  end
+
+  #Returns the module the call is inside
+  def module
+    expect :result
+
+    self[1]
+  end
+
+  #Return the class the call is inside
+  def result_class
+    expect :result
+
+    self[2]
+  end
 end
 
 #Invalidate hash cache if the Sexp changes

test/apps/rails3/app/controllers/other_controller.rb

@@ -48,4 +48,8 @@ class OtherController < ApplicationController
     `#{some_command}`
     system("ls #{some_files}")
   end
+
+  def test_mass_assign_with_strong_params
+    Bill.create(params[:charge])
+  end
 end

test/apps/rails3/app/models/bill.rb

+class Bill < ActiveRecord::Base
+  include ActiveModel::ForbiddenAttributesProtection
+end

test/tests/test_mass_assign_disable.rb

@@ -36,4 +36,30 @@ class MassAssignDisableTest < Test::Unit::TestCase
       end
     RUBY
   end
+
+  def test_strong_parameters_in_initializer
+    init = "config/initializers/mass_assign.rb"
+    gemfile = "Gemfile"
+    config = "config/application.rb"
+
+    before_rescan_of [init, gemfile, config], "rails3.2" do
+      write_file init, <<-RUBY
+        class ActiveRecord::Base
+          include ActiveModel::ForbiddenAttributesProtection
+        end
+      RUBY
+
+      append gemfile, "gem 'strong_parameters'"
+
+      replace config, "config.active_record.whitelist_attributes = true",
+        "config.active_record.whitelist_attributes = false"
+    end
+
+    #We disable whitelist, but add strong_parameters globally, so
+    #there should be no change.
+    assert_reindex :none
+    assert_changes
+    assert_fixed 0
+    assert_new 0
+  end
 end

test/tests/test_rails3.rb

@@ -589,6 +589,15 @@ class Rails3Tests < Test::Unit::TestCase
       :file => /account\.rb/
   end
 
+  def test_mass_assign_with_strong_params
+    assert_no_warning :type => :warning,
+      :warning_type => "Mass Assignment",
+      :line => 53,
+      :message => /^Unprotected\ mass\ assignment/,
+      :confidence => 0,
+      :file => /other_controller\.rb/
+  end
+
   def test_translate_bug
     assert_warning :type => :warning,
       :warning_type => "Cross Site Scripting",