Namespaced classes that are not fully qualified can cause difference in false positives/negatives (WIP)

lib/brakeman/checks/base_check.rb

@@ -432,14 +432,12 @@ def has_immediate_model? exp, out = nil
   #the current tracker, or else @models should contain an array of the model
   #names, which is available via tracker.models.keys
   def model_name? exp
-    @models ||= @tracker.models.keys
-
     if exp.is_a? Symbol
-      @models.include? exp
+      tracker.models.include? exp
     elsif call? exp and exp.target.nil? and exp.method == :current_user
       true
     elsif sexp? exp
-      @models.include? class_name(exp)
+      tracker.models.include? class_name(exp)
     else
       false
     end
@@ -485,11 +483,11 @@ def self.description
   def active_record_models
     return @active_record_models if @active_record_models
 
-    @active_record_models = {}
+    @active_record_models = Brakeman::ClassCollection.new
 
-    tracker.models.each do |name, model|
+    tracker.models.each_class do |model|
       if model.ancestor? :"ActiveRecord::Base"
-        @active_record_models[name] = model
+        @active_record_models << model
       end
     end
 

lib/brakeman/checks/check_detailed_exceptions.rb

@@ -24,7 +24,7 @@ def check_local_request_config
   end
 
   def check_detailed_exceptions
-    tracker.controllers.each do |_name, controller|
+    tracker.controllers.each_class do |controller|
       controller.methods_public.each do |method_name, definition|
         src = definition[:src]
         body = src.body.last

lib/brakeman/checks/check_forgery_setting.rb

@@ -14,11 +14,11 @@ def run_check
 
     tracker.controllers
     .select { |_, controller| controller.parent == :"ActionController::Base" }
-    .each do |name, controller|
+    .each do |_, controller|
       if controller and not controller.protect_from_forgery?
-        csrf_warning :controller => name,
+        csrf_warning :controller => controller.name,
           :warning_code => :csrf_protection_missing,
-          :message => msg(msg_code("protect_from_forgery"), " should be called in ", msg_code(name)),
+          :message => msg(msg_code("protect_from_forgery"), " should be called in ", msg_code(controller.name)),
           :file => controller.file,
           :line => controller.top_line
       elsif version_between? "4.0.0", "100.0.0" and forgery_opts = controller.options[:protect_from_forgery]
@@ -27,7 +27,7 @@ def run_check
           access_arg.value == :exception
 
           args = {
-            :controller => name,
+            :controller => controller.name,
             :warning_type => "Cross-Site Request Forgery",
             :warning_code => :csrf_not_protected_by_raising_exception,
             :message => msg(msg_code("protect_from_forgery"), " should be configured with ", msg_code("with: :exception")),

lib/brakeman/checks/check_model_serialize.rb

@@ -17,7 +17,7 @@ def run_check
 
     return unless @upgrade_version
 
-    tracker.models.each do |_name, model|
+    tracker.models.each_class do |model|
       check_for_serialize model
     end
   end

lib/brakeman/checks/check_nested_attributes.rb

@@ -29,7 +29,7 @@ def run_check
   end
 
   def uses_nested_attributes?
-    active_record_models.each do |_name, model|
+    active_record_models.each_class do |model|
       return true if model.options[:accepts_nested_attributes_for]
     end
 

lib/brakeman/checks/check_nested_attributes_bypass.rb

@@ -18,7 +18,7 @@ def run_check
   end
 
   def check_nested_attributes
-    active_record_models.each do |name, model|
+    active_record_models.each_class do |model|
       if opts = model.options[:accepts_nested_attributes_for]
         opts.each do |args|
           if args.any? { |a| allow_destroy? a } and args.any? { |a| reject_if? a }

lib/brakeman/checks/check_page_caching_cve.rb

@@ -10,17 +10,13 @@ def run_check
     gem_version = tracker.config.gem_version(gem_name.to_sym)
     upgrade_version = '1.2.2'
     cve = 'CVE-2020-8159'
-
     return unless gem_version and version_between?('0.0.0', '1.2.1', gem_version)
-
     message = msg("Directory traversal vulnerability in ", msg_version(gem_version, gem_name), " ", msg_cve(cve), ". Upgrade to ", msg_version(upgrade_version, gem_name))
-
     if uses_caches_page?
       confidence = :high
     else
       confidence = :weak
     end
-
     warn :warning_type => 'Directory Traversal',
       :warning_code => :CVE_2020_8159,
       :message => message,
@@ -30,8 +26,9 @@ def run_check
   end
 
   def uses_caches_page?
-    tracker.controllers.any? do |name, controller|
-      controller.options.has_key? :caches_page
+    tracker.controllers.each_class do |controller|
+      return true if controller.options[:caches_page]
     end
+    return false
   end
 end

lib/brakeman/checks/check_skip_before_filter.rb

@@ -13,7 +13,7 @@ class Brakeman::CheckSkipBeforeFilter < Brakeman::BaseCheck
   @description = "Warn when skipping CSRF or authentication checks by default"
 
   def run_check
-    tracker.controllers.each do |_name, controller|
+    tracker.controllers.each_class do |controller|
       controller.skip_filters.each do |filter|
         process_skip_filter filter, controller
       end

lib/brakeman/checks/check_sql.rb

@@ -97,7 +97,7 @@ def find_scope_calls
   end
 
   def ar_scope_calls(symbol_name, &block)
-    active_record_models.each do |name, model|
+    active_record_models.each_class do |model|
       model_args = model.options[symbol_name]
       if model_args
         model_args.each do |args|

lib/brakeman/checks/check_validation_regex.rb

@@ -16,7 +16,7 @@ class Brakeman::CheckValidationRegex < Brakeman::BaseCheck
   FORMAT = Sexp.new(:lit, :format)
 
   def run_check
-    active_record_models.each do |name, model|
+    active_record_models.each_class do |model|
       @current_model = model
       format_validations = model.options[:validates_format_of]
 

lib/brakeman/processors/lib/module_helper.rb

@@ -32,7 +32,7 @@ def handle_module exp, tracker_class, parent = nil
 
   def handle_class exp, collection, tracker_class
     name = class_name(exp.class_name)
-    parent = class_name exp.parent_name
+    parent = class_name(exp.parent_name)
 
     if @current_class
       outer_class = @current_class
@@ -43,12 +43,14 @@ def handle_class exp, collection, tracker_class
       name = (@current_module.name.to_s + "::" + name.to_s).to_sym
     end
 
-    if collection[name]
-      @current_class = collection[name]
+    bm_name = Brakeman::ClassName.new(name)
+
+    if collection[bm_name]
+      @current_class = collection[bm_name]
       @current_class.add_file @current_file, exp
     else
-      @current_class = tracker_class.new name, parent, @current_file, exp, @tracker
-      collection[name] = @current_class
+      @current_class = tracker_class.new bm_name, parent, @current_file, exp, @tracker
+      collection[bm_name] = @current_class
     end
 
     exp.body = process_all! exp.body

lib/brakeman/rescanner.rb

@@ -205,7 +205,7 @@ def rescan_lib path
 
     lib = nil
 
-    tracker.libs.each do |_name, library|
+    tracker.libs.each_class do |library|
       if library.files.include?(path)
         lib = library
         break
@@ -277,7 +277,7 @@ def rescan_deleted_template path
   def rescan_deleted_lib path
     deleted_lib = nil
 
-    tracker.libs.delete_if do |_name, lib|
+    tracker.libs.delete_if do |lib|
       if lib.files.include?(path)
         deleted_lib = lib
         true
@@ -297,7 +297,7 @@ def remove_deleted_file path
     deleted = false
 
     [:controllers, :models, :libs].each do |collection|
-      tracker.send(collection).delete_if do |_name, data|
+      tracker.send(collection).delete_if do |data|
         if data.files.include?(path)
           deleted = true
           true
@@ -349,7 +349,7 @@ def rescan_mixin lib
     to_rescan = []
 
     #Rescan controllers that mixed in library
-    tracker.controllers.each do |_name, controller|
+    tracker.controllers.each_class do |controller|
       if controller.includes.include? lib.name
         controller.files.each do |path|
           unless @paths.include? path

lib/brakeman/tracker.rb

@@ -6,6 +6,7 @@
 require 'brakeman/processors/lib/find_all_calls'
 require 'brakeman/tracker/config'
 require 'brakeman/tracker/constants'
+require 'brakeman/tracker/class_collection'
 
 #The Tracker keeps track of all the processed information.
 class Brakeman::Tracker
@@ -16,7 +17,7 @@ class Brakeman::Tracker
 
   #Place holder when there should be a model, but it is not
   #clear what model it will be.
-  UNKNOWN_MODEL = :BrakemanUnresolvedModel
+  UNKNOWN_MODEL = Brakeman::ClassName.new(:BrakemanUnresolvedModel)
 
   #Creates a new Tracker.
   #
@@ -29,16 +30,16 @@ def initialize(app_tree, processor = nil, options = {})
 
     @config = Brakeman::Config.new(self)
     @templates = {}
-    @controllers = {}
+    @controllers = Brakeman::ClassCollection.new
     #Initialize models with the unknown model so
     #we can match models later without knowing precisely what
     #class they are.
-    @models = {}
+    @models = Brakeman::ClassCollection.new
     @models[UNKNOWN_MODEL] = Brakeman::Model.new(UNKNOWN_MODEL, nil, @app_tree.file_path("NOT_REAL.rb"), nil, self)
     @routes = {}
     @initializers = {}
     @errors = []
-    @libs = {}
+    @libs = Brakeman::ClassCollection.new
     @constants = Brakeman::Constants.new
     @checks = nil
     @processed = nil