Merge branch 'ctaintor/improve_fingerprint'

Make the attr_accessible check warning fingerprint include the method name

CHANGES

+# Unrelease
+
+ * Fingerprint attribute warnings individually (Case Taintor)
+
 # 2.3.1
 
  * Fix check for CVE-2013-4491 (i18n XSS) to detect workaround

lib/brakeman/checks/check_model_attr_accessible.rb

@@ -29,8 +29,9 @@ class Brakeman::CheckModelAttrAccessible < Brakeman::BaseCheck
               :file => model[:file],
               :warning_type => "Mass Assignment",
               :warning_code => :dangerous_attr_accessible,
-              :message => "Potentially dangerous attribute '#{attribute}' available for mass assignment",
-              :confidence => confidence
+              :message => "Potentially dangerous attribute available for mass assignment",
+              :confidence => confidence,
+              :code => Sexp.new(:lit, attribute)
             break # Prevent from matching single attr multiple times
           end
         end

test/apps/rails4/app/models/account.rb

 class Account < ActiveRecord::Base
-  attr_accessible :name
+  attr_accessible :name, :account_id, :admin
 end

test/apps/rails4/config/brakeman.ignore

+{
+  "ignored_warnings": [
+    {
+      "warning_type": "Mass Assignment",
+      "warning_code": 60,
+      "fingerprint": "cd83ecf615b17f849ba28050e7faf1d54f218dfa9435c3f65f47cb378c18cf98",
+      "message": "Potentially dangerous attribute available for mass assignment",
+      "file": "app/models/account.rb",
+      "line": null,
+      "link": "http://brakemanscanner.org/docs/warning_types/mass_assignment/",
+      "code": ":admin",
+      "render_path": null,
+      "location": {
+        "type": "model",
+        "model": "Account"
+      },
+      "user_input": null,
+      "confidence": "High",
+      "note": "skipping this for a test"
+    }
+  ],
+  "updated": "2013-12-20 22:14:42 +0200",
+  "brakeman_version": "2.3.1"
+}

test/tests/rails31.rb

@@ -1075,7 +1075,7 @@ class Rails31Tests < Test::Unit::TestCase
       :warning_code => 17,
       :fingerprint => "77c353ad8e5fc9880775ed436bbfa37b005b43aa2978186de92b6916f46fac39",
       :warning_type => "Mass Assignment",
-      :message => /^Potentially\ dangerous\ attribute\ admin\ av/,
+      :message => "Potentially dangerous attribute available for mass assignment: :admin",
       :confidence => 0,
       :relative_path => "app/models/user.rb"
   end
@@ -1085,7 +1085,7 @@ class Rails31Tests < Test::Unit::TestCase
       :warning_code => 60,
       :fingerprint => "e933f99c33bece852891a466b5b0fc629d9f20ba80ff3bbc42adfd239d5a5b48",
       :warning_type => "Mass Assignment",
-      :message => /^Potentially\ dangerous\ attribute\ 'blah_admin/,
+      :message => "Potentially dangerous attribute available for mass assignment: :blah_admin_blah",
       :confidence => 0,
       :relative_path => "app/models/account.rb"
   end

test/tests/rails32.rb

@@ -252,45 +252,48 @@ class Rails32Tests < Test::Unit::TestCase
     assert_warning :type => :model,
       :warning_code => 60,
       :warning_type => "Mass Assignment",
-      :message => /^Potentially\ dangerous\ attribute\ 'admin'/,
+      :message => "Potentially dangerous attribute available for mass assignment: :admin",
       :confidence => 0, #HIGH
       :file => /user\.rb/
-  end 
+  end
 
   def test_model_attr_accessible_account_id
     assert_warning :type => :model,
       :warning_code => 60,
-      :fingerprint => "1d6615676c39afae6d749891e45d7351423542b3fe71a6eaf088bf7573e5c4b0",
+      :fingerprint => "add78ac0c12cea9335ad3128f17fd0ff8b0f3772daca1d0d109f9dc02ea2df59",
       :warning_type => "Mass Assignment",
-      :message => /^Potentially\ dangerous\ attribute\ 'account_id'/,
+      :message => "Potentially dangerous attribute available for mass assignment: :account_id",
       :confidence => 0,
       :relative_path => "app/models/user.rb"
-  end 
+  end
 
   def test_model_attr_accessible_account_banned
     assert_warning :type => :model,
       :warning_code => 60,
       :warning_type => "Mass Assignment",
-      :message => /^Potentially\ dangerous\ attribute\ 'banned'/,
+      :message => "Potentially dangerous attribute available for mass assignment: :banned",
       :confidence => 1, #MED
       :file => /account\.rb/
-  end 
+  end
 
   def test_model_attr_accessible_status_id
     assert_warning :type => :model,
       :warning_code => 60,
       :warning_type => "Mass Assignment",
-      :message => /^Potentially\ dangerous\ attribute\ 'status_id'/,
+      :message => "Potentially dangerous attribute available for mass assignment: :status_id",
       :confidence => 2, #LOW
       :file => /user\.rb/
-  end 
+  end
 
   def test_model_attr_accessible_plan_id
     assert_warning :type => :model,
       :warning_type => "Mass Assignment",
-      :message => /^Potentially\ dangerous\ attribute\ 'plan_id'/,
+      :message => "Potentially dangerous attribute available for mass assignment: :plan_id",
       :confidence => 2, 
       :file => /account\.rb/
-  end 
+  end
 
+  def test_two_distinct_warnings_cant_have_same_fingerprint
+    assert_equal report[:model_warnings].map(&:fingerprint), report[:model_warnings].map(&:fingerprint).uniq
+  end
 end

test/tests/rails4.rb

@@ -13,7 +13,7 @@ class Rails4Tests < Test::Unit::TestCase
   def expected
     @expected ||= {
       :controller => 0,
-      :model => 0,
+      :model => 1,
       :template => 1,
       :generic => 12
     }
@@ -241,4 +241,26 @@ class Rails4Tests < Test::Unit::TestCase
       :relative_path => "app/controllers/friendly_controller.rb",
       :user_input => nil
   end
+
+  def test_only_desired_attribute_is_ignored
+    assert_warning :type => :model,
+      :warning_code => 60,
+      :fingerprint => "e543ea9186ed27e78ccfeee4e60ceee0c83163ffe0bf50e1ebf3d7b19793c5f4",
+      :warning_type => "Mass Assignment",
+      :line => nil,
+      :message => "Potentially dangerous attribute available for mass assignment: :account_id",
+      :confidence => 0,
+      :relative_path => "app/models/account.rb",
+      :user_input => nil
+
+    assert_no_warning :type => :model,
+      :warning_code => 60,
+      :fingerprint => "cd83ecf615b17f849ba28050e7faf1d54f218dfa9435c3f65f47cb378c18cf98",
+      :warning_type => "Mass Assignment",
+      :line => nil,
+      :message => "Potentially dangerous attribute available for mass assignment: :admin",
+      :confidence => 0,
+      :relative_path => "app/models/account.rb",
+      :user_input => nil
+  end
 end

test/tests/rails4_with_engines.rb

@@ -198,10 +198,10 @@ class Rails4WithEnginesTests < Test::Unit::TestCase
   def test_mass_assignment_12
     assert_warning :type => :model,
       :warning_code => 60,
-      :fingerprint => "18df17e4364b62c4ba1c6e2849f8302592c68d196ab43f753639f9043c1e4014",
+      :fingerprint => "dbb51200329e5eadf073c7145497d0b18e33d903248426b6e8b97ec5d03ec23a",
       :warning_type => "Mass Assignment",
       #noline,
-      :message => /^Potentially\ dangerous\ attribute\ 'plan_id/,
+      :message => "Potentially dangerous attribute available for mass assignment: :plan_id",
       :confidence => 2,
       :relative_path => "engines/user_removal/app/models/account.rb"
   end
@@ -209,10 +209,10 @@ class Rails4WithEnginesTests < Test::Unit::TestCase
   def test_mass_assignment_13
     assert_warning :type => :model,
       :warning_code => 60,
-      :fingerprint => "e2fb5b0d650caf257ef86e32b101f9488738388e91039cc130c365a8df9b83fb",
+      :fingerprint => "c505002e3567c74c8197586751d0cf9ab245aee0068f05c93589959b14dc40c8",
       :warning_type => "Mass Assignment",
       #noline,
-      :message => /^Potentially\ dangerous\ attribute\ 'banned'/,
+      :message => "Potentially dangerous attribute available for mass assignment: :banned",
       :confidence => 1,
       :relative_path => "engines/user_removal/app/models/account.rb"
   end
@@ -220,10 +220,10 @@ class Rails4WithEnginesTests < Test::Unit::TestCase
   def test_mass_assignment_14
     assert_warning :type => :model,
       :warning_code => 60,
-      :fingerprint => "6276c85369c13ed06f18ca1dd9a7ef076077154e98f0c29b7938b5649a7d115d",
+      :fingerprint => "962a14c66f5f83ece9a22700939111a0b71ed2c925980416f1b664a601e87070",
       :warning_type => "Mass Assignment",
       #noline,
-      :message => /^Potentially\ dangerous\ attribute\ 'account/,
+      :message => "Potentially dangerous attribute available for mass assignment: :account_id",
       :confidence => 0,
       :relative_path => "engines/user_removal/app/models/user.rb"
   end
@@ -231,10 +231,10 @@ class Rails4WithEnginesTests < Test::Unit::TestCase
   def test_mass_assignment_15
     assert_warning :type => :model,
       :warning_code => 60,
-      :fingerprint => "6276c85369c13ed06f18ca1dd9a7ef076077154e98f0c29b7938b5649a7d115d",
+      :fingerprint => "fa154c3e50c02c70f4351dd6731085657dfb0b9ed73ee223ad5444b31bc1d31f",
       :warning_type => "Mass Assignment",
       #noline,
-      :message => /^Potentially\ dangerous\ attribute\ 'admin'\ /,
+      :message => "Potentially dangerous attribute available for mass assignment: :admin",
       :confidence => 0,
       :relative_path => "engines/user_removal/app/models/user.rb"
   end
@@ -242,10 +242,10 @@ class Rails4WithEnginesTests < Test::Unit::TestCase
   def test_mass_assignment_16
     assert_warning :type => :model,
       :warning_code => 60,
-      :fingerprint => "6fd655a6dcf618e378d5f7e7b3a9c038ed9b29d66ab89f9c28343265b2ff6d75",
+      :fingerprint => "98c24601f549d41e0d0367e8bcefc6083263fa175a2978ace0340c6446e57603",
       :warning_type => "Mass Assignment",
       #noline,
-      :message => /^Potentially\ dangerous\ attribute\ 'status_/,
+      :message => "Potentially dangerous attribute available for mass assignment: :status_id",
       :confidence => 2,
       :relative_path => "engines/user_removal/app/models/user.rb"
   end