diff --git a/lib/brakeman/checks/check_model_attr_accessible.rb b/lib/brakeman/checks/check_model_attr_accessible.rb index 34128b88ebbc1f8616a0db802ba35836476fb8b3..e7fab64cab91eed1c248307816b6cb35c4806042 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