cdelafuente-r7 commented 4 years ago

Target DefaultOptions are not imported into the datastore when module is initialized, but only if you explicitly set the target. Apparently, this only works if import_target_defaults is added at the end of the initialize method.

Steps to reproduce

Create this module modules/exploits/windows/import_target_defaults_test.rb


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

class MetasploitModule < Msf::Exploit::Remote Rank = NormalRanking

include Exploit::Remote::Tcp

def initialize(info = {}) super( update_info( info, 'Name' => 'Sample Exploit', 'Description' => %q( foo ), 'License' => MSF_LICENSE, 'Author' => ['skape'], 'Targets' => [

Target 0: Windows All

        [
          'Windows XP/Vista/7/8',
          {
            'Platform' => 'win',
            'Ret'      => 0x41424344,
            'DefaultOptions' => { 'DisablePayloadHandler' => true, 'WfsDelay' => 0 }
          }
        ],
        [
          'Windows 10',
          {
            'Platform' => 'win',
            'Ret'      => 0x45454545,
            'DefaultOptions' => { 'DisablePayloadHandler' => false, 'WfsDelay' => 90 }
          }
        ]
      ],
    'DefaultTarget'  => 0
  )
)
#import_target_defaults

end

def exploit return end end

2. `./msfconsole`
3. `msf5 > use exploits/windows/import_target_defaults_test`
4. Target 0 is the default target:
```ruby
msf5 exploit(windows/import_target_defaults_test) > show options

Module options (exploit/windows/import_target_defaults_test):

   Name    Current Setting  Required  Description
   ----    ---------------  --------  -----------
   RHOSTS                   yes       The target host(s), range CIDR identifier, or hosts file with syntax 'file:<path>'
   RPORT                    yes       The target port (TCP)

Exploit target:

   Id  Name
   --  ----
   0   Windows XP/Vista/7/8

The advanced options DisablePayloadHandler and WfsDelay are not set as expected
```
msf5 exploit(windows/import_target_defaults_test) > show advanced
```

Module advanced options (exploit/windows/import_target_defaults_test):

Name Current Setting Required Description

CHOST no The local client address CPORT no The local client port ConnectTimeout 10 yes Maximum number of seconds to establish a TCP connection ContextInformationFile no The information file that contains context information DisablePayloadHandler false no Disable the handler code for the selected payload EnableContextEncoding false no Use transient context when encoding payloads Proxies no A proxy chain of format type:host:port[,type:host:port][...] SSL false no Negotiate SSL/TLS for outgoing connections SSLCipher no String for SSL cipher - "DHE-RSA-AES256-SHA" or "ADH" SSLVerifyMode PEER no SSL verification method (Accepted: CLIENT_ONCE, FAIL_IF_NO_PEER_CERT, NONE, PEER) SSLVersion Auto yes Specify the version of SSL/TLS to be used (Auto, TLS and SSL23 are auto-negotiate) (Accepted: Auto, TLS, SSL23, SSL3, TLS1, TLS1.1, TLS1.2) VERBOSE false no Enable detailed status messages WORKSPACE no Specify the workspace for this module WfsDelay 0 no Additional delay when waiting for a session

6. even after executing the exploit, these options are still not set correctly:
```ruby
msf5 exploit(windows/import_target_defaults_test) > set rhosts 172.16.60.173
rhosts => 172.16.60.173
msf5 exploit(windows/import_target_defaults_test) > set rport 25
rport => 25
msf5 exploit(windows/import_target_defaults_test) > run

[*] Started reverse TCP handler on 172.16.60.1:4444
[*] Exploit completed, but no session was created.
msf5 exploit(windows/import_target_defaults_test) > show advanced

Module advanced options (exploit/windows/import_target_defaults_test):

   Name                    Current Setting  Required  Description
   ----                    ---------------  --------  -----------
   CHOST                                    no        The local client address
   CPORT                                    no        The local client port
   ConnectTimeout          10               yes       Maximum number of seconds to establish a TCP connection
   ContextInformationFile                   no        The information file that contains context information
   DisablePayloadHandler   false            no        Disable the handler code for the selected payload
   EnableContextEncoding   false            no        Use transient context when encoding payloads
   Proxies                                  no        A proxy chain of format type:host:port[,type:host:port][...]
   SSL                     false            no        Negotiate SSL/TLS for outgoing connections
   SSLCipher                                no        String for SSL cipher - "DHE-RSA-AES256-SHA" or "ADH"
   SSLVerifyMode           PEER             no        SSL verification method (Accepted: CLIENT_ONCE, FAIL_IF_NO_PEER_CERT, NONE, PEER)
   SSLVersion              Auto             yes       Specify the version of SSL/TLS to be used (Auto, TLS and SSL23 are auto-negotiate) (Accepted: Auto, TLS, SSL23, SSL3, TLS1, TLS1.1, TLS1.2)
   VERBOSE                 false            no        Enable detailed status messages
   WORKSPACE                                no        Specify the workspace for this module
   WfsDelay                0                no        Additional delay when waiting for a session

The DefaultOptions are correctly imported if you explicitly set the target:


msf5 exploit(windows/import_target_defaults_test) > set target 1
target => 1
msf5 exploit(windows/import_target_defaults_test) > show advanced

Module advanced options (exploit/windows/import_target_defaults_test):

Name Current Setting Required Description

CHOST no The local client address CPORT no The local client port ConnectTimeout 10 yes Maximum number of seconds to establish a TCP connection ContextInformationFile no The information file that contains context information DisablePayloadHandler false no Disable the handler code for the selected payload EnableContextEncoding false no Use transient context when encoding payloads Proxies no A proxy chain of format type:host:port[,type:host:port][...] SSL false no Negotiate SSL/TLS for outgoing connections SSLCipher no String for SSL cipher - "DHE-RSA-AES256-SHA" or "ADH" SSLVerifyMode PEER no SSL verification method (Accepted: CLIENT_ONCE, FAIL_IF_NO_PEER_CERT, NONE, PEER) SSLVersion Auto yes Specify the version of SSL/TLS to be used (Auto, TLS and SSL23 are auto-negotiate) (Accepted: Auto, TLS, SSL23, SSL3, TLS1, TLS1.1, TLS1.2) VERBOSE false no Enable detailed status messages WORKSPACE no Specify the workspace for this module WfsDelay 90 no Additional delay when waiting for a session



## Expected behavior
Target DefaultOptions should be imported into the datastore when the module is initialized.

wvu commented 4 years ago

Something may be overriding this:

https://github.com/rapid7/metasploit-framework/blob/df277e14064fb0dd8966cf0b552483274fad78af/lib/msf/core/exploit.rb#L380-L381

I've tried moving it to the bottom of the method to no avail, so it's something else.

dwelch-r7 commented 4 years ago

Interesting I wasn't able to replicate with modules/exploits/linux/http/hp_van_sdn_cmd_inject.rb until I added in some of the options you were testing, like WfsDelay the PAYLOAD option being set in the default options worked just fine, I'll continue to look into it

wvu commented 4 years ago

FYI, this still works with PAYLOAD for me, just not anything else. :/

wvu commented 4 years ago

This is also affecting unset all!

github-actions[bot] commented 4 years ago

Hi!

This issue has been left open with no activity for a while now.

We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 30 days since the last update here. If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open!

As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request.

adfoster-r7 commented 3 years ago

TLDR: It looks like this is caused by the tcp mixin calling register_options, which doesn't take into consideration the target's requested default values.

If I understand correctly this is caused by the the mixin pattern and the initialize method's use of:

super(
   update_info(
      ...
   )
)

First update_info is called first, resulting in:

{
  "Name": "Sample Exploit",
  "Description": "\n          foo\n        ",
  "License": "Metasploit Framework License (BSD)",
  "Author": [
    "skape"
  ],
  "Targets": [
    [
      "Windows XP/Vista/7/8",
      {
        "Platform": "win",
        "Ret": 1094861636,
        "DefaultOptions": {
          "DisablePayloadHandler": true,
          "WfsDelay": 0
        }
      }
    ],
    [
      "Windows 10",
      {
        "Platform": "win",
        "Ret": 1162167621,
        "DefaultOptions": {
          "DisablePayloadHandler": false,
          "WfsDelay": 90
        }
      }
    ]
  ],
  "DefaultTarget": 0
}

Then the result is passed to super, which will go through the various ancestors (assuming super is called) consecutively:

self.class.ancestors
=> [Msf::Modules::Auxiliary__Scanner__Smb__Smb_enumshares::MetasploitModule,
 Msf::Exploit::Remote::Tcp,
 Msf::Exploit::Remote,
 Msf::Exploit::AutoTarget,
 Msf::Exploit,
 Msf::Module,
 Msf::Module::Reliability,
 Msf::Module::Stability,
 Msf::Module::SideEffects,
 Msf::Module::UUID,
 Msf::Module::UI,
 Msf::Module::UI::Message,
 Msf::Module::UI::Message::Verbose,
 Msf::Module::UI::Line,
 Msf::Module::UI::Line::Verbose,
 Rex::Ui::Subscriber,
 Rex::Ui::Subscriber::Input,
 Rex::Ui::Subscriber::Output,
 Msf::Module::Type,
 Msf::Module::Ranking,
 Msf::Module::Privileged,
 Msf::Module::Options,
 Msf::Module::Network,
 Msf::Module::ModuleStore,
 Msf::Module::ModuleInfo,
 Msf::Module::FullName,
 Msf::Module::DataStore,
 Msf::Module::Compatibility,
 Msf::Module::Author,
 Msf::Module::Auth,
 Msf::Module::Arch,
 Msf::Module::Alert,
 ActiveSupport::ToJsonWithActiveSupportEncoder,
 Object,
 ActiveSupport::Dependencies::Loadable,
 BSON::Object,
 SNMP::BER,
 WindowsError::Win32,
 PCAPRUB,
 ActiveSupport::Tryable,
 JSON::Ext::Generator::GeneratorMethods::Object,
 Rex::Arch,
 ERB::Util,
 PP::ObjectMixin,
 Kernel,
 BasicObject]

The super call eventually reaches msf/core/exploit.rb which imports targets, sets defaults. overrides WslDelay:

    self.targets = Rex::Transformer.transform(info['Targets'], Array, [ Target ], 'Targets')

    ....

    import_target_defaults

    ...

    # All exploits can increase the delay when waiting for a session.
    # However, this only applies to aggressive exploits.
    if aggressive?
      register_advanced_options(
        [
          OptInt.new('WfsDelay', [ false, "Additional delay when waiting for a session", 0 ])
        ], Msf::Exploit)
    end

Placing import_target_defaults at the end of the exploit initialize method doesn't work in this scenario. When this method returns, the tcp mixin calls register_option which resets the option values back to their defaults values from the info object - ignoring the target defaults:

    52:   # vulnerability in a TCP server.
    53:   #
    54:   def initialize(info = {})
    55:     super <---------------------- Parent correctly sets options to match default target options
    56: 
 => 57:     register_options( <--------- Overrides the values back to the default options, rather than target options
    58:       [
    59:         Opt::RHOST,
    60:         Opt::RPORT
    61:       ], Msf::Exploit::Remote::Tcp
    62:     )

Under the hood, it seems like register_options just imports the default values again when calling import_options with self.options and doesn't take into consideration the values that were set by import_target_defaults

    64: def register_options(options, owner = self.class)
    65:   self.options.add_options(options, owner)
 => 66:   self.datastore.import_options(self.options, 'self', true)
    67:   import_defaults(false)
    68: end

Initially I believe a quick solution for this scenario seems like placing import_target_defaults at the end of the exploit file, as well as at the end of the tcp server mixin. I don't know what ramifications this might have on other modules, or the effort of investigating if the fix should live in register_options in some shape - and what impact that fix would have. It would require more investigation.

wvu commented 3 years ago

Thanks for taking the time to look into this!

rapid7 / metasploit-framework

Target DefaultOptions not imported into the datastore when module is initialized #12963

Steps to reproduce

Target 0: Windows All