Verify datastore option validation in all scenarios

[dwelch-r7]

We need to verify if when calling a module from another module does the datastore option validation happen, if not then we need to ensure that the validation does occur

There may be other scenarios where the validation doesn't run too those should also be addressed if they come up in the process of working on this issue

[smcintyre-r7]

The issue I noticed specifically is that if a child module has a datastore option registered by a mixin that the child module de-registers and then relies on being nil/blank in the control flow provided by the mixin, there is no guarantee that the option is absent from the datastore options the child module is invoked with. This would be the case when a parent module invokes it and includes the same mixin.

To address the problem, I'd propose that datastore options are filtered before being passed to the underlying module to prevent values that are not registered from being forwarded.

`auxiliary/test/child_required_option`

This module deregisters `HttpUsername` and `HttpPassword` for whatever reason and the module author assumes that it'll be `#blank?` in both their code as well as the HttpClient mixin's code.

##
# This module requires Metasploit: https://metasploit.com/download
# Current source: https://github.com/rapid7/metasploit-framework
##

class MetasploitModule < Msf::Auxiliary
  include Msf::Exploit::Remote::HttpClient

  def initialize(info = {})
    super(
      update_info(
        info,
        'Name' => 'Child HTTP Module (No Auth)',
        'Description' => 'Test child module that includes HttpClient but deregisters
                          HttpUsername and HttpPassword because it targets an
                          unauthenticated endpoint. Expects those options to be
                          nil/blank so connect() does not send credentials.',
        'Author' => ['unknown'],
        'License' => MSF_LICENSE,
        'Notes' => {
          'Stability' => [],
          'Reliability' => [],
          'SideEffects' => []
        }
      )
    )

    deregister_options('HttpUsername', 'HttpPassword')
  end

  def run
    username = datastore['HttpUsername']
    password = datastore['HttpPassword']

    if username.blank? && password.blank?
      print_good('HttpUsername and HttpPassword are absent (deregister worked)')
      print_good('connect() would NOT send HTTP auth credentials')
    else
      print_warning("HttpUsername = #{username.inspect}")
      print_warning("HttpPassword = #{password.inspect}")
      print_warning('connect() would send these leaked credentials for HTTP auth!')
    end
  end
end

`auxiliary/text/parent_run_api`

##
# This module requires Metasploit: https://metasploit.com/download
# Current source: https://github.com/rapid7/metasploit-framework
##

class MetasploitModule < Msf::Auxiliary
  include Msf::Exploit::Remote::HttpClient

  def initialize(info = {})
    super(
      update_info(
        info,
        'Name' => 'Parent HTTP Module (Leaks Deregistered Options)',
        'Description' => 'Test parent module that includes HttpClient and invokes
                          a child module which also includes HttpClient but has
                          deregistered HttpUsername/HttpPassword. Demonstrates that
                          deregistered option values leak to the child.',
        'Author' => ['unknown'],
        'License' => MSF_LICENSE,
        'Notes' => {
          'Stability' => [],
          'Reliability' => [],
          'SideEffects' => []
        }
      )
    )
  end

  def run
    child_refname = 'test/child_required_option'

    print_status("Parent HttpUsername = #{datastore['HttpUsername'].inspect}")
    print_status("Parent HttpPassword = #{datastore['HttpPassword'].inspect}")

    # ---- TEST 1: Copy parent's options to child ----
    # Both modules include HttpClient. The parent iterates its own options
    # and copies values to the child -- a realistic pattern when both
    # modules need to talk to the same HTTP target. The parent has no way
    # to know the child deregistered HttpUsername/HttpPassword.
    print_status('')
    print_status('=' * 60)
    print_status('TEST 1: Parent copies its own options to child')
    print_status('=' * 60)
    begin
      child = framework.modules.create(child_refname)
      unless child
        print_error("Failed to create module: #{child_refname}")
        return
      end
      self.options.each_pair do |name, _opt|
        next if datastore[name].nil?

        child.datastore[name] = datastore[name]
      end
      child.run_simple(
        'LocalInput' => user_input,
        'LocalOutput' => user_output,
        'RunAsJob' => false
      )
    rescue ::Exception => e
      print_error("TEST 1 exception: #{e.class} - #{e.message}")
    end

    # ---- TEST 2: Global datastore leak ----
    # No explicit copy -- the child's ModuleDataStore falls back to the
    # framework global datastore, leaking any globally-set values.
    print_status('')
    print_status('=' * 60)
    print_status('TEST 2: Global datastore leak (no explicit copy)')
    print_status('=' * 60)
    begin
      old_user = framework.datastore['HttpUsername']
      old_pass = framework.datastore['HttpPassword']
      framework.datastore['HttpUsername'] = 'global_admin'
      framework.datastore['HttpPassword'] = 'global_secret'

      child = framework.modules.create(child_refname)
      unless child
        print_error("Failed to create module: #{child_refname}")
        return
      end
      child.datastore['RHOSTS'] = datastore['RHOSTS']
      child.datastore['RPORT'] = datastore['RPORT']
      child.run_simple(
        'LocalInput' => user_input,
        'LocalOutput' => user_output,
        'RunAsJob' => false
      )
    rescue ::Exception => e
      print_error("TEST 2 exception: #{e.class} - #{e.message}")
    ensure
      framework.datastore['HttpUsername'] = old_user
      framework.datastore['HttpPassword'] = old_pass
    end

    # ---- TEST 3: Clean invocation (control) ----
    # No leaked values -- child should see nil/blank.
    print_status('')
    print_status('=' * 60)
    print_status('TEST 3: Clean invocation (no leaked values)')
    print_status('=' * 60)
    begin
      child = framework.modules.create(child_refname)
      unless child
        print_error("Failed to create module: #{child_refname}")
        return
      end
      child.datastore['RHOSTS'] = datastore['RHOSTS']
      child.datastore['RPORT'] = datastore['RPORT']
      child.run_simple(
        'LocalInput' => user_input,
        'LocalOutput' => user_output,
        'RunAsJob' => false
      )
    rescue ::Exception => e
      print_error("TEST 3 exception: #{e.class} - #{e.message}")
    end
  end
end

With these two test modules you can see the options are leaked by the validation routine, causing #blank? checks to fail when the module author probably thought they would always be true.

msf auxiliary(scanner/postgres/postgres_login) > use auxiliary/test/child_required_option 
msf auxiliary(test/child_required_option) > show options 

Module options (auxiliary/test/child_required_option):

   Name     Current Setting  Required  Description
   ----     ---------------  --------  -----------
   Proxies                   no        A proxy chain of format type:host:port[,type:host:port][...]. Supported proxies: http, sapni, socks4, socks5, socks5h
   RHOSTS                    yes       The target host(s), see https://docs.metasploit.com/docs/using-metasploit/basics/using-metasploit.html
   RPORT    80               yes       The target port (TCP)
   SSL      false            no        Negotiate SSL/TLS for outgoing connections
   VHOST                     no        HTTP server virtual host


View the full module info with the info, or info -d command.

msf auxiliary(test/child_required_option) > set RHOSTS 127.0.0.1
RHOSTS => 127.0.0.1
msf auxiliary(test/child_required_option) > run
[*] Running module against 127.0.0.1
[+] HttpUsername and HttpPassword are absent (deregister worked)
[+] connect() would NOT send HTTP auth credentials
[*] Auxiliary module execution completed
msf auxiliary(test/child_required_option) > use auxiliary/test/parent_run_api 
msf auxiliary(test/parent_run_api) > set RHOSTS 127.0.0.1
RHOSTS => 127.0.0.1
msf auxiliary(test/parent_run_api) > set HttpUsername spencer
HttpUsername => spencer
msf auxiliary(test/parent_run_api) > set HttpPassword Password1!
HttpPassword => Password1!
msf auxiliary(test/parent_run_api) > run
[*] Running module against 127.0.0.1
[*] Parent HttpUsername = "spencer"
[*] Parent HttpPassword = "Password1!"
[*] 
[*] ============================================================
[*] TEST 1: Parent copies its own options to child
[*] ============================================================
[!] HttpUsername = "spencer"
[!] HttpPassword = "Password1!"
[!] connect() would send these leaked credentials for HTTP auth!
[*] 
[*] ============================================================
[*] TEST 2: Global datastore leak (no explicit copy)
[*] ============================================================
[!] HttpUsername = "global_admin"
[!] HttpPassword = "global_secret"
[!] connect() would send these leaked credentials for HTTP auth!
[*] 
[*] ============================================================
[*] TEST 3: Clean invocation (no leaked values)
[*] ============================================================
[+] HttpUsername and HttpPassword are absent (deregister worked)
[+] connect() would NOT send HTTP auth credentials
[*] Auxiliary module execution completed
msf auxiliary(test/parent_run_api) >

Options that are registered seem to be validated so long as one of the _simple methods are used, e.g. #run_simple, #check_simple, etc.

Modules I found that don't use the datastore-validated API:

Module	Line	Call	Link
modules/auxiliary/admin/http/netgear_pnpx_getsharefolderlist_auth_bypass.rb	251	handler.run	permalink
modules/post/multi/recon/local_exploit_suggester.rb	218	mod[:module].check	permalink
modules/post/multi/recon/persistence_suggester.rb	242	mod[:module].check	permalink