Add check for permitted mass assignment attributes

lib/brakeman/checks/check_permit_attributes.rb

@@ -0,0 +1,43 @@
+require 'brakeman/checks/base_check'
+
+class Brakeman::CheckPermitAttributes < Brakeman::BaseCheck
+  Brakeman::Checks.add self
+
+  @description = "Warn on potentially dangerous attributes whitelisted via permit"
+
+  SUSPICIOUS_KEYS = {
+    admin: :high,
+    account_id: :high,
+    role: :medium,
+    banned: :medium,
+  }
+
+  def run_check
+    tracker.find_call(:method => :permit).each do |result|
+      check_permit result
+    end
+  end
+
+  def check_permit result
+    call = result[:call]
+
+    call.each_arg do |arg|
+      if symbol? arg
+        if SUSPICIOUS_KEYS.key? arg.value
+          warn_on_permit_key result, arg
+        elsif arg.value.match /_id$/
+          warn_on_permit_key result, arg, :medium
+        end
+      end
+    end
+  end
+
+  def warn_on_permit_key result, key, confidence = nil
+    warn :result => result,
+      :warning_type => "Mass Assignment",
+      :warning_code => :dangerous_permit_key,
+      :message => "Potentially dangerous key allowed for mass assignment",
+      :confidence => (confidence || SUSPICIOUS_KEYS[key.value]),
+      :user_input => key
+  end
+end

lib/brakeman/warning_codes.rb

@@ -106,6 +106,7 @@ module Brakeman::WarningCodes
     :CVE_2016_6316 => 102,
     :CVE_2016_6317 => 103,
     :divide_by_zero => 104,
+    :dangerous_permit_key => 105,
   }
 
   def self.code name

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

@@ -85,6 +85,11 @@ def guard_with_return
   def render_cookies
     render inline: request.cookies["value"]
   end
+
+  def dangerous_permits
+    params.permit(:admin)
+    params.permit(:role_id)
+  end
 end
 
 IDENTIFIER_NAMESPACE = 'apis'

test/tests/rails5.rb

@@ -13,7 +13,7 @@ def expected
       :controller => 0,
       :model => 0,
       :template => 9,
-      :generic => 15
+      :generic => 17
     }
   end
 
@@ -43,6 +43,32 @@ def test_mass_assignment_with_slice
       :user_input => nil
   end
 
+  def test_mass_assignment_permit_high
+    assert_warning :type => :warning,
+      :warning_code => 105,
+      :fingerprint => "615627822842388859734b6124bf99e0db057a2572f35c92ff42b5ad46f4415f",
+      :warning_type => "Mass Assignment",
+      :line => 90,
+      :message => /^Potentially\ dangerous\ key\ allowed\ for\ ma/,
+      :confidence => 0,
+      :relative_path => "app/controllers/widget_controller.rb",
+      :code => s(:call, s(:params), :permit, s(:lit, :admin)),
+      :user_input => s(:lit, :admin)
+  end
+
+  def test_mass_assignment_permit_medium
+    assert_warning :type => :warning,
+      :warning_code => 105,
+      :fingerprint => "c4c89a39b0a2dc707027f47747312d27308ea219a009e4f0116a759a71ad561b",
+      :warning_type => "Mass Assignment",
+      :line => 91,
+      :message => /^Potentially\ dangerous\ key\ allowed\ for\ ma/,
+      :confidence => 1,
+      :relative_path => "app/controllers/widget_controller.rb",
+      :code => s(:call, s(:params), :permit, s(:lit, :role_id)),
+      :user_input => s(:lit, :role_id)
+  end
+
   def test_sql_injection_with_slice
     assert_no_warning :type => :warning,
       :warning_code => 0,