Improves deregistering datastore options validation

Author: cgranleese-r7

lib/msf/core/module_data_store.rb

@@ -15,6 +15,7 @@ def initialize(m)
       super()
 
       @_module = m
+      @deregistered_keys = []
     end
 
     #
@@ -27,6 +28,56 @@ def copy
       new_instance
     end
 
+  # Grab any available deregistered_keys, otherwise return a blank array
+  #
+  # @param [Msf::DataStore] other The other datastore to copy state from
+  # @return [Msf::DataStore] the current datastore instance
+    def copy_state(other)
+      super
+      incoming = other.respond_to?(:deregistered_keys, true) ? other.deregistered_keys : []
+      incoming.each do |key|
+        @deregistered_keys << key unless @deregistered_keys.any? { |k| k.casecmp?(key) }
+      end
+
+      # Strip any deregistered keys that may have been written into @user_defined
+      # by the parent's copy_state (e.g. during reverse_merge! which calls
+      # copy_state with a plain DataStore result that contains all merged values).
+      @deregistered_keys.each do |key|
+        k = find_key_case(key)
+        @user_defined.delete(k)
+      end
+
+      # Need self here to return datastore instance from parent
+      self
+    end
+
+    # Track deregistered keys
+    #
+    # @param [String] name
+    # @return [nil]
+    def remove_option(name)
+      super
+      @deregistered_keys << name
+      
+      # Need nil here to return original nil value from parent
+      nil
+    end
+
+    # Imports options and clears any matching keys from the deregistered list.
+    # This allows a module to call deregister_options followed by register_options
+    # for the same key and ensure we remove the re-registered option from the
+    # tracked @deregistered_keys
+    #
+    # @param [Msf::OptionContainer] options
+    # @param [String, nil] imported_by
+    # @param [Boolean] overwrite
+    def import_options(options, imported_by = nil, overwrite = true)
+      options.each_option do |name, _option|
+        @deregistered_keys.delete_if { |k| k.casecmp?(name) }
+      end
+      super
+    end
+
     # Search for a value within the current datastore, taking into consideration any registered aliases, fallbacks, etc.
     # If a value is not present in the current datastore, the global parent store will be referenced instead
     #
@@ -36,6 +87,9 @@ def search_for(key)
       k = find_key_case(key)
       return search_result(:user_defined, @user_defined[k]) if @user_defined.key?(k)
 
+      # If the module has not registered the key then return early as it has been deregistered
+      return search_result(:not_found, nil) if should_filter_key?(key)
+
       # Preference globally set values over a module's option default
       framework_datastore_search = search_framework_datastore(key)
       return framework_datastore_search if framework_datastore_search.found? && !framework_datastore_search.default?
@@ -61,8 +115,57 @@ def search_for(key)
       search_framework_datastore(k)
     end
 
+    # Override the write path so that keys which were explicitly deregistered on
+    # the owning module are dropped.
+    #
+    # Compatible and functions as normal when:
+    #   - the key has never been deregistered via deregister_options
+    #   - there is no associated module (@_module is nil)
+    #
+    # @param [String] key
+    # @param [Object] value
+    def []=(key, value)
+      return if should_filter_key?(key)
+
+      super
+    end
+
+    # Override merge! so that when merging a DataStore (which writes directly to
+    # @user_defined, bypassing []=), any deregistered keys are stripped out
+    # afterward. This prevents a caller from injecting a deregistered option's
+    # value by merging a datastore that contains it.
+    #
+    # When merging a plain Hash the parent's merge! already routes through []=,
+    # so deregistered keys are filtered at that point and no extra work is needed.
+    #
+    # @param [Msf::DataStore, Hash] other
+    # @return [Msf::ModuleDataStore]
+    def merge!(other)
+      super
+      if other.is_a?(DataStore)
+        @deregistered_keys.each do |key|
+          k = find_key_case(key)
+          @user_defined.delete(k)
+          @options.delete_if { |opt_name, _| opt_name.casecmp?(k) }
+          @aliases.delete_if { |_, v| v.casecmp?(k) }
+        end
+      end
+      self
+    end
+
     protected
 
+    attr_reader :deregistered_keys
+
+    # Returns true when the write should be silently dropped because the key
+    # was explicitly deregistered via deregister_options.
+    #
+    # @param [String] key
+    # @return [Boolean]
+    def should_filter_key?(key)
+      @deregistered_keys.any? { |deregistered| deregistered.casecmp?(key) }
+    end
+
     # Search the framework datastore
     #
     # @param [String] key The key to search for