Convert MSSQL mixin to class #18696

zgoldman-r7 · 2024-01-12T22:13:10Z

This continues the work started in #18615

Before adding an MSSQL session type, we need a standalone client class, in addition to a mixin. This PR consolidates the two instances of MSSQL client classes, consolidates them, and converts the consolidated mixin into a class that all existing MSSQL modules have been tweaked to work with.

To test:

Boot up an MSSQL instance, and take note of the machine's ip
load up any modules changed in this pr. Set rhost to the target's ip, rport to the port MSSQL is running on (usually 1433), username, password, and any other required options.
run the module
compare behavior to master. Nothing should have changed

lib/metasploit/framework/mssql/client_mixin.rb

lib/metasploit/framework/mssql/client.rb

lib/metasploit/framework/mssql/metasploit_mixin.rb

lib/metasploit/framework/login_scanner/mssql.rb

modules/auxiliary/admin/mssql/mssql_enum.rb

lib/rex/proto/mssql/client.rb

modules/auxiliary/admin/mssql/mssql_ntlm_stealer.rb

modules/exploits/windows/mssql/ms09_004_sp_replwritetovarbin.rb

spec/lib/metasploit/framework/login_scanner/mssql_spec.rb

adfoster-r7 · 2024-01-30T20:50:31Z

lib/metasploit/framework/tcp/client.rb

@@ -93,7 +93,7 @@ def connect(global = true, opts={})
              'SSLCipher'     =>  opts['SSLCipher'] || ssl_cipher,
              'Proxies'       => proxies,
              'Timeout'       => (opts['ConnectTimeout'] || connection_timeout || 10).to_i,
-              'Context'       => { 'Msf' => framework, 'MsfExploit' => framework_module }
+              'Context'       => { 'Msf' => framework, 'MsfExploit' => self }


is there any context on this change? 👀

Let me revert and re-test and get back to you, this might be fine to revert

I assume it was fine to revert? 😄

You got it!

modules/auxiliary/scanner/mssql/mssql_hashdump.rb

adfoster-r7 · 2024-01-30T21:11:03Z

modules/auxiliary/scanner/mssql/mssql_hashdump.rb

@@ -70,7 +70,7 @@ def run_host(ip)
    create_credential_login(login_data)

    # Grabs the Instance Name and Version of MSSQL(2k,2k5,2k8)
-    instancename= mssql_query(mssql_enumerate_servername())[:rows][0][0].split('\\')[1]
+    instancename= mssql_query(mssql_enumerate_servername())[:rows][0][0].split('\\')[0]


Hm, it looks like this [1] was intentionally added previously:

a5e63c2

I wonder if whatever your current string returns needed to be handled, as well as whatever value was previously expected?

Interesting - IIRC the [1] was returning NULL at times even in master with [0] having the data we were looking for, I'll verify whether this is the right change or something I need to adjust elsewhere

running this on both my branch and master, mssql_query(mssql_enumerate_servername())[:rows][0][0].split('\\') returns ["WIN-F0OBFMVGB07"] - so the result of running this with [1] gives us [*] 192.168.2.212:1433 - Instance Name: nil on the output. Maybe I've got something configured wrong, but either way it seems like this should be tweaked to either [0] or there's some sort of logic we need to wrap this in

I wonder if the behaviour should be preferencing second part, and if it's not there - fallback to the using the first part? 🤔 Worth a check 👍

modules/auxiliary/scanner/mssql/mssql_ping.rb

adfoster-r7 · 2024-01-30T21:12:47Z

modules/exploits/windows/mssql/lyris_listmanager_weak_pass.rb

-      ))
+    ))
+    register_options([
+      OptBool.new('DISPLAY_RESULTS', [true, "Display the Results to the Screen", true])


Why was this change needed? 👀

I think this (and the above) snuck in during a merge, I'll see if I can nuke

modules/exploits/windows/mssql/mssql_clr_payload.rb

adfoster-r7 · 2024-01-31T01:28:13Z

Looks like this breaks kerberos auth:

msf6 auxiliary(scanner/mssql/mssql_login) > run 192.168.123.132 domaincontrollerrhost=192.168.123.132 username=vagrant password=vagrant mssql::auth=kerberos mssql::rhostname=dc01.demo.local mssqldomain=demo.local sql='select auth_scheme from sys.dm_exec_connections where session_id=@@spid'

[*] 192.168.123.132:1433 - MSSQL - Starting authentication scanner.
[!] No active DB -- Credential data will not be saved!
[-] 192.168.123.132:1433 - LOGIN FAILED: WORKSTATION\vagrant:vagrant (Unable to Connect: undefined local variable or method `hostname' for #<Rex::Proto::MSSQL::Client:0x00007f7a7151f930 @framework_module=#<Module:auxiliary/scanner/mssql/mssql_login datastore=[#<Msf::ModuleDataStoreWithFallbacks:0x00007f7a5111e748 @options={"WORKSPACE"=>#<Msf::OptString:0x00007f7a612c3de0 @name="WORKSPACE", @advanced=true, @evasion=false, @aliases=[], @max_length=nil, @conditions=[], @fallbacks=[], @required=false, @desc="Specify the workspace for this module", @default=nil, @enums=[], @owner=Msf::Module>, "VERBOSE"=>#<Msf::OptBool:0x00007f7a612...etc...

Stack trace:

Call stack:
/Users/user/Documents/code/metasploit-framework/lib/rex/proto/mssql/client.rb:105:in `mssql_login'
/Users/user/Documents/code/metasploit-framework/lib/metasploit/framework/login_scanner/mssql.rb:72:in `attempt_login'
/Users/user/Documents/code/metasploit-framework/lib/metasploit/framework/login_scanner/base.rb:231:in `block in scan!'
/Users/user/Documents/code/metasploit-framework/lib/metasploit/framework/login_scanner/base.rb:163:in `block in each_credential'
/Users/user/Documents/code/metasploit-framework/lib/metasploit/framework/credential_collection.rb:89:in `block in each_filtered'
/Users/user/Documents/code/metasploit-framework/lib/metasploit/framework/credential_collection.rb:249:in `each_unfiltered'
/Users/user/Documents/code/metasploit-framework/lib/metasploit/framework/credential_collection.rb:86:in `each_filtered'
/Users/user/Documents/code/metasploit-framework/lib/metasploit/framework/login_scanner/base.rb:141:in `each_credential'
/Users/user/Documents/code/metasploit-framework/lib/metasploit/framework/login_scanner/base.rb:205:in `scan!'
/Users/user/Documents/code/metasploit-framework/modules/auxiliary/scanner/mssql/mssql_login.rb:84:in `run_host'
/Users/user/Documents/code/metasploit-framework/lib/msf/core/auxiliary/scanner.rb:128:in `block (2 levels) in run'
/Users/user/Documents/code/metasploit-framework/lib/msf/core/thread_manager.rb:105:in `block in spawn'

adfoster-r7 · 2024-01-31T01:50:46Z

It looks like this is leaking file descriptors/not closing sockets.

You can verify this with lsof -p msfconsole_pid_id_1234 with the amount of sockets it has opened currently

Run the mssql query module multiple times:

msf6 auxiliary(admin/mssql/mssql_sql) > repeat -n 30 run rhost=192.168.123.132 username=vagrant password=vagrant use_windows_authent=true sql='select auth_scheme from sys.dm_exec_connections where session_id=@@spid'

See that the sockets are not closed and are leaking:

$ lsof -p 70509
COMMAND   PID     USER   FD   TYPE             DEVICE  SIZE/OFF      NODE NAME
...
ruby    70509 user   18u  IPv4 0xebd80902b209bf9d       0t0       TCP 192.168.123.1:51181->192.168.123.132:ms-sql-s (ESTABLISHED)
ruby    70509 user   19u  IPv4 0xebd80902b079d31d       0t0       TCP 192.168.123.1:51167->192.168.123.132:ms-sql-s (ESTABLISHED)
ruby    70509 user   20u  IPv4 0xebd80902b07ac7fd       0t0       TCP 192.168.123.1:51121->192.168.123.132:ms-sql-s (ESTABLISHED)
ruby    70509 user   21u  IPv4 0xebd80902a675a95d       0t0       TCP 192.168.123.1:51065->192.168.123.132:ms-sql-s (ESTABLISHED)
ruby    70509 user   22u  IPv4 0xebd80902b0781abd       0t0       TCP 192.168.123.1:51170->192.168.123.132:ms-sql-s (ESTABLISHED)
ruby    70509 user   23u  IPv4 0xebd80902b0780f9d       0t0       TCP 192.168.123.1:51172->192.168.123.132:ms-sql-s (ESTABLISHED)
ruby    70509 user   24u  IPv4 0xebd80902a677347d       0t0       TCP 192.168.123.1:51174->192.168.123.132:ms-sql-s (ESTABLISHED)
ruby    70509 user   25u  IPv4 0xebd80902b07825dd       0t0       TCP 192.168.123.1:51176->192.168.123.132:ms-sql-s (ESTABLISHED)
ruby    70509 user   26u  IPv4 0xebd80902b077e31d       0t0       TCP 192.168.123.1:51177->192.168.123.132:ms-sql-s (ESTABLISHED)
ruby    70509 user   27u  IPv4 0xebd80902b077c1bd       0t0       TCP 192.168.123.1:51178->192.168.123.132:ms-sql-s (ESTABLISHED)
ruby    70509 user   28u  IPv4 0xebd80902b077ee3d       0t0       TCP 192.168.123.1:51179->192.168.123.132:ms-sql-s (ESTABLISHED)
ruby    70509 user   29u  IPv4 0xebd80902b07aff9d       0t0       TCP 192.168.123.1:51081->192.168.123.132:ms-sql-s (ESTABLISHED)
ruby    70509 user   30u  IPv4 0xebd80902b077d7fd       0t0       TCP 192.168.123.1:51169->192.168.123.132:ms-sql-s (ESTABLISHED)
ruby    70509 adfoster   31u  IPv4 0xebd80902b07af47d       0t0       TCP 192.168.123.1:51138->192.168.123.132:ms-sql-s (ESTABLISHED)
ruby    70509 user   32u  IPv4 0xebd80902a676295d       0t0       TCP 192.168.123.1:51168->192.168.123.132:ms-sql-s (ESTABLISHED)

But if we compare to master, there's no leaks

adfoster-r7 · 2024-01-31T09:59:07Z

Test RC file I was running with resource resoure.rc

<ruby>
auth_modules = %w[
  auxiliary/scanner/mssql/mssql_login
  auxiliary/scanner/mssql/mssql_hashdump
  auxiliary/scanner/mssql/mssql_ping
  auxiliary/scanner/mssql/mssql_schemadump
  exploit/windows/mssql/mssql_clr_payload
  auxiliary/admin/mssql/mssql_exec
  auxiliary/admin/mssql/mssql_enum
  exploit/windows/mssql/mssql_linkcrawler
  auxiliary/admin/mssql/mssql_escalate_dbowner
  auxiliary/admin/mssql/mssql_escalate_execute_as
  auxiliary/admin/mssql/mssql_findandsampledata
  auxiliary/admin/mssql/mssql_sql
  auxiliary/admin/mssql/mssql_sql_file
  auxiliary/admin/mssql/mssql_idf
  exploit/windows/mssql/mssql_payload
  exploit/windows/mssql/mssql_payload_sqli
  auxiliary/admin/mssql/mssql_escalate_dbowner_sqli
  auxiliary/admin/mssql/mssql_escalate_execute_as_sqli
  auxiliary/admin/mssql/mssql_enum_domain_accounts_sqli
  auxiliary/admin/mssql/mssql_enum_sql_logins
  auxiliary/admin/mssql/mssql_enum_domain_accounts

  post/windows/gather/credentials/mssql_local_hashdump
  post/windows/manage/mssql_local_auth_bypass
]

auth_modules.each do |mod|
  print_line
  print_status("Running mod :: #{mod}")
  run_single("use #{mod}")
  if mod.start_with?('auxiliary') || mod.include?('exploit')
    # Windows auth
    run_single("run rhost=192.168.123.132 username=vagrant password=vagrant rport=1433 use_windows_authent=true lhost=192.168.123.1")
    # Normal auth
    run_single("run rhost=192.168.123.132 username=admin password=p4$$w0rd rport=1433 use_windows_authent=false lhost=192.168.123.1")
    # Kerberos auth
    # run_single("run 192.168.123.132 domaincontrollerrhost=192.168.123.132 username=vagrant password=vagrant mssql::auth=kerberos mssql::rhostname=dc01.demo.local mssqldomain=demo.local sql='select auth_scheme from sys.dm_exec_connections where session_id=@@spid'")
  elsif mod.start_with?('post')
    run_single("run session=-1")
  else
    raise "Unknown mod #{mod}"
  end
  print_line
end
</ruby>

dwelch-r7 · 2024-01-31T14:22:45Z

lib/rex/proto/mssql/client.rb

+        attr_accessor :use_ntlmv2
+        attr_accessor :windows_authentication
+        attr_reader :framework_module
+        attr_reader :framework


what uses this framework attribute? strikes me as a little odd for a client of any kind to require a framework object
potentially the same question for framework_module I know it's being used to pull stuff out of the datastore, but is the object itself needed and/or can we just pass an opts hash instead?
asking out of curiosity more than anything else, not a blocker

Primarily, both of these get used in mssql_login - open to alternative implementations though!

where in mssql_login? I can't see it being accessed through the client, the mssql_login module provides the scanner with the framework and framework_module though

Sorry - the mssql_login method in lib/rex/proto/mssql/client.rb. The two main cases are:

framework_module.fail_with(Msf::Exploit::Failure::BadConfig, 'The Mssql::Rhostname option is required when using kerberos authentication.') if @hostname.blank? kerberos_authenticator = Msf::Exploit::Remote::Kerberos::ServiceAuthenticator::MSSQL.new( host: domain_controller_rhost, hostname: @hostname, mssql_port: rport, proxies: proxies, realm: domain_name, username: user, password: pass, framework: framework, framework_module: framework_module, ticket_storage: Msf::Exploit::Remote::Kerberos::Ticket::Storage::WriteOnly.new(framework: framework, framework_module: framework_module) )

and in using print_status/print_error
Should I investigate a way around it?

ah I see it now, I didn't realise it was in code you didn't change

dwelch-r7 · 2024-01-31T14:31:54Z

modules/auxiliary/scanner/mssql/mssql_login.rb

+    register_options(
+      [
+        Opt::RHOST,
+        Opt::RPORT(1433),
+        OptString.new('USERNAME', [ false, 'The username to authenticate as', 'sa']),
+        OptString.new('PASSWORD', [ false, 'The password for the specified username', '']),
+        OptBool.new('TDSENCRYPTION', [ true, 'Use TLS/SSL for TDS data "Force Encryption"', false]),
+        OptBool.new('USE_WINDOWS_AUTHENT', [ true, 'Use windows authentication (requires DOMAIN option set)', false]),
+      ])


This change doesn't seem right, shouldn't there be a mixin pulling these in (or at least the common ones like RHOST and RPORT) for the mssql modules?
Similar to the smb_login module here still includes a mixin registering these options but makes use of an SMB client too

this module runs differently since it's coming from the login_scanner rather than the client directly, but I'll investigate pulling this out, that's definitely the cleaner approach

dwelch-r7 · 2024-01-31T14:32:35Z

modules/auxiliary/scanner/mssql/mssql_schemadump.rb

-    instancename = mssql_query(mssql_enumerate_servername())[:rows][0][0].split('\\')[1]
+    instancename = mssql_query(mssql_enumerate_servername())[:rows][0][0].split('\\')[0]


this a bug you found/fixed?

I believe so - see #18696 (comment)

adfoster-r7 · 2024-02-01T16:27:39Z

lib/metasploit/framework/login_scanner/mssql.rb

-        validates :tdsencryption,
-          inclusion: { in: [true, false] }
+        # validates :tdsencryption, - TODO: support TDS Encryption
+        #   inclusion: { in: [true, false] }


It smells like this is fixing a side effect rather than the underlying issue

i.e. Does the caller need fixed, rather than this validation being removed?

adfoster-r7 · 2024-02-01T16:31:50Z

lib/msf/core/exploit/remote/mssql.rb

@@ -104,622 +99,140 @@ def mssql_ping_parse(data)
    return res
  end

+  def disconnect


This will break any other disconnect implementations inherited from a super class. To avoid doing that bug, we should be calling super here when using mixins like this.

To take a step back though, I believe the real issue is you're not registering the sock with the framework module for it to be automatically cleaned up here:

Main branch:

msf6 auxiliary(admin/mssql/mssql_sql) > run 192.168.123.132 domaincontrollerrhost=192.168.123.132 username=vagrant password=vagrant mssql::auth=kerberos mssql::rhostname=dc01.demo.local mssqldomain=demo.local sql='select auth_scheme from sys.dm_exec_connections where session_id=@@spid' [*] Running module against 192.168.123.132 From: /Users/adfoster/Documents/code/metasploit-framework/lib/msf/core/auxiliary.rb:135 Msf::Auxiliary#add_socket: 133: def add_socket(sock) 134: require 'pry-byebug'; binding.pry => 135: self.sockets << sock 136: end [1] pry(#<Msf::Modules::Auxiliary__Admin__Mssql__Mssql_sql::MetasploitModule>)> sock => #<Socket:fd 15> [2] pry(#<Msf::Modules::Auxiliary__Admin__Mssql__Mssql_sql::MetasploitModule>)>

If we followed the existing pattern of registering the socket, we wouldn't need this custom disconnect method here - or I believe you could register the socket still, ensure you call super here, and ensure you've got a nil check with @client.disconnect if @client

adfoster-r7 · 2024-02-01T16:44:22Z

Looks like Kerberos is still broken even after the kerberos fixes commit 👀

msf6 auxiliary(admin/mssql/mssql_sql) > run 192.168.123.132 domaincontrollerrhost=192.168.123.132 username=vagrant password=vagrant mssql::auth=kerberos mssql::rhostname=dc01.demo.local mssqldomain=demo.local sql='select auth_scheme from sys.dm_exec_connections where session_id=@@spid'
[*] Running module against 192.168.123.132

[-] 192.168.123.132:1433 - Auxiliary failed: NameError undefined local variable or method `domain_controller_rhost' for #<Rex::Proto::MSSQL::Client:0x00007fc3c1f7cb78 @framework_module=#<Module:auxiliary/admin/mssql/mssql_sql datastore=[#<Msf::ModuleDataStoreWithFallbacks:0x00007fc3c1e2eb40 @options={"WORKSPACE"=>#<Msf::OptString:0x00007fc3c1a46780 @name="WORKSPACE", @advanced=true, @evasion=false, @aliases=[], @max_length=nil, @conditions=[], @fallbacks=[], @required=false, @desc="Specify the workspace for this module", @default=nil, @enums=[], @owner=Msf::Module>, "VERBOSE"=>#<Msf::OptBool:0x00007fc3c1a46578 @name="VERBOSE", @advanced=true, @evasion=false, @aliases=[], @max_length=nil, @conditions=[], @fallbacks=[], @required=false, @desc="Enable detailed status messa

And the crash in other modules:

msf6 auxiliary(scanner/mssql/mssql_ping) > run rhost=192.168.123.132

[*] 192.168.123.132:      - SQL Server information for 192.168.123.132:
[+] 192.168.123.132:      -    ServerName      = DC01
[+] 192.168.123.132:      -    InstanceName    = SQLEXPRESS
[+] 192.168.123.132:      -    IsClustered     = No
[+] 192.168.123.132:      -    Version         = 16.0.1000.6
[+] 192.168.123.132:      -    tcp             = 1433
[+] 192.168.123.132:      -    np              = \\DC01\pipe\MSSQL$SQLEXPRESS\sql\query
[-] 192.168.123.132:      - Auxiliary failed: NoMethodError undefined method `disconnect' for nil:NilClass
[-] 192.168.123.132:      - Call stack:
[-] 192.168.123.132:      -   /Users/user/Documents/code/metasploit-framework/lib/msf/core/exploit/remote/mssql.rb:103:in `disconnect'
[-] 192.168.123.132:      -   /Users/user/Documents/code/metasploit-framework/lib/msf/core/exploit/remote/tcp.rb:204:in `cleanup'
[-] 192.168.123.132:      -   /Users/user/Documents/code/metasploit-framework/lib/msf/core/auxiliary/scanner.rb:142:in `block (2 levels) in run'
[-] 192.168.123.132:      -   /Users/user/Documents/code/metasploit-framework/lib/msf/core/thread_manager.rb:105:in `block in spawn'
[-] Auxiliary failed: NoMethodError undefined method `disconnect' for nil:NilClass
[-] Call stack:
[-]   /Users/user/Documents/code/metasploit-framework/lib/msf/core/exploit/remote/mssql.rb:103:in `disconnect'
[-]   /Users/user/Documents/code/metasploit-framework/lib/msf/core/exploit/remote/tcp.rb:204:in `cleanup'

resource.rc

modules/auxiliary/scanner/mssql/mssql_login.rb

convert first module from remote to client move client to rex remove metasploit mixin

adfoster-r7 · 2024-02-02T14:15:06Z

Release Notes

Introduces a standalone MSSQL client class that can be used in new contexts not tied to a specific module

zgoldman-r7 commented Jan 12, 2024

View reviewed changes

lib/metasploit/framework/mssql/client_mixin.rb Outdated Show resolved Hide resolved

zgoldman-r7 commented Jan 12, 2024

View reviewed changes

lib/metasploit/framework/mssql/client.rb Outdated Show resolved Hide resolved

zgoldman-r7 commented Jan 12, 2024

View reviewed changes

lib/metasploit/framework/mssql/metasploit_mixin.rb Outdated Show resolved Hide resolved

zgoldman-r7 force-pushed the convert-mssql-to-class branch 2 times, most recently from b548572 to e443438 Compare January 18, 2024 02:51

zgoldman-r7 marked this pull request as ready for review January 18, 2024 02:55

zgoldman-r7 force-pushed the convert-mssql-to-class branch from 11a20f6 to 6d4edb4 Compare January 18, 2024 03:12

adfoster-r7 reviewed Jan 18, 2024

View reviewed changes

lib/metasploit/framework/login_scanner/mssql.rb Show resolved Hide resolved

adfoster-r7 reviewed Jan 18, 2024

View reviewed changes

modules/auxiliary/admin/mssql/mssql_enum.rb Outdated Show resolved Hide resolved

zgoldman-r7 force-pushed the convert-mssql-to-class branch from 6d4edb4 to 9fa145e Compare January 18, 2024 15:28

bwatters-r7 reviewed Jan 18, 2024

View reviewed changes

lib/rex/proto/mssql/client.rb Outdated Show resolved Hide resolved

modules/auxiliary/admin/mssql/mssql_ntlm_stealer.rb Outdated Show resolved Hide resolved

modules/exploits/windows/mssql/ms09_004_sp_replwritetovarbin.rb Outdated Show resolved Hide resolved

zgoldman-r7 mentioned this pull request Jan 18, 2024

MS9177 MSSQL part 1 - client consolidation #18615

Closed

zgoldman-r7 force-pushed the convert-mssql-to-class branch from 164a28e to 448725f Compare January 18, 2024 20:52

zgoldman-r7 commented Jan 19, 2024

View reviewed changes

spec/lib/metasploit/framework/login_scanner/mssql_spec.rb Show resolved Hide resolved

zgoldman-r7 mentioned this pull request Jan 25, 2024

Mssql session type #18747

Merged

12 tasks