From ad1c7ad719baff71eed65463b7f1f22ee85dee28 Mon Sep 17 00:00:00 2001 From: Justin Collins Date: Wed, 21 Aug 2013 11:23:13 -0700 Subject: [PATCH] Make ModelAttrAccessible check consistent in 1.8 --- .../checks/check_model_attr_accessible.rb | 45 +++++++++---------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/lib/brakeman/checks/check_model_attr_accessible.rb b/lib/brakeman/checks/check_model_attr_accessible.rb index 34128b88..e7fab64c 100644 --- a/lib/brakeman/checks/check_model_attr_accessible.rb +++ b/lib/brakeman/checks/check_model_attr_accessible.rb @@ -1,39 +1,38 @@ require 'brakeman/checks/base_check' -# Author: Paul Deardorff (themetric) -# Checks models to see if important foreign keys -# or attributes are exposed as attr_accessible when -# they probably shouldn't be. +# Author: Paul Deardorff (themetric) +# Checks models to see if important foreign keys +# or attributes are exposed as attr_accessible when +# they probably shouldn't be. class Brakeman::CheckModelAttrAccessible < Brakeman::BaseCheck Brakeman::Checks.add self @description = "Reports models which have dangerous attributes defined under the attr_accessible whitelist." - SUSP_ATTRS = { - /admin/ => CONFIDENCE[:high], # Very dangerous unless some Rails authorization used - /role/ => CONFIDENCE[:med], - /banned/ => CONFIDENCE[:med], - :account_id => CONFIDENCE[:high], - /\S*_id(s?)\z/ => CONFIDENCE[:low] # All other foreign keys have weak/low confidence - } + SUSP_ATTRS = [ + [/admin/, CONFIDENCE[:high]], # Very dangerous unless some Rails authorization used + [/role/, CONFIDENCE[:med]], + [/banned/, CONFIDENCE[:med]], + [:account_id, CONFIDENCE[:high]], + [/\S*_id(s?)\z/, CONFIDENCE[:low]] # All other foreign keys have weak/low confidence + ] def run_check check_models do |name, model| accessible_attrs = model[:attr_accessible] accessible_attrs.each do |attribute| SUSP_ATTRS.each do |susp_attr, confidence| - if susp_attr.is_a?(Regexp) and susp_attr =~ attribute.to_s or susp_attr == attribute - warn :model => name, - :file => model[:file], - :warning_type => "Mass Assignment", - :warning_code => :mass_assign_call, - :message => "Potentially dangerous attribute #{attribute} available for mass assignment.", - :confidence => confidence - break # Prevent from matching single attr multiple times - end - end - end + if susp_attr.is_a?(Regexp) and susp_attr =~ attribute.to_s or susp_attr == attribute + warn :model => name, + :file => model[:file], + :warning_type => "Mass Assignment", + :message => "Potentially dangerous attribute #{attribute} available for mass assignment.", + :confidence => confidence + break # Prevent from matching single attr multiple times + end + end + end end end @@ -44,5 +43,5 @@ class Brakeman::CheckModelAttrAccessible < Brakeman::BaseCheck end end end - + end -- GitLab