rapid7 / metasploit-framework

Metasploit Framework
https://www.metasploit.com/
Other
32.92k stars 13.72k forks source link

Better enforce types to prevent nil values from causing stack traces #19114

Closed zeroSteiner closed 3 weeks ago

zeroSteiner commented 3 weeks ago

This PR fixes a number of nil-type related stack traces that crept in as we were adding features to our LDAP work.

  1. This fixes the return value of a few methods in the MsIcpr mixin to return an empty array instead of nil as the documentation states it should.

This fixes an exception reported to me by @bwatters-r7

msf6 auxiliary(admin/dcerpc/icpr_cert) > run
[*] Running module against 192.168.108.211

[+] 3.19.108.211:445 - The requested certificate was issued.
[-] 3.19.108.211:445 - Auxiliary failed: NoMethodError undefined method `empty?' for nil:NilClass
[-] 3.19.108.211:445 - Call stack:
[-] 3.19.108.211:445 -   /home/tmoose/rapid7/metasploit-framework/lib/msf/core/exploit/remote/ms_icpr.rb:214:in `do_request_cert'
[-] 3.19.108.211:445 -   /home/tmoose/rapid7/metasploit-framework/lib/msf/core/exploit/remote/ms_icpr.rb:97:in `request_certificate'
[-] 3.19.108.211:445 -   /home/tmoose/rapid7/metasploit-framework/modules/auxiliary/admin/dcerpc/icpr_cert.rb:68:in `block in action_request_cert'
[-] 3.19.108.211:445 -   /home/tmoose/rapid7/metasploit-framework/modules/auxiliary/admin/dcerpc/icpr_cert.rb:83:in `with_ipc_tree'
[-] 3.19.108.211:445 -   /home/tmoose/rapid7/metasploit-framework/modules/auxiliary/admin/dcerpc/icpr_cert.rb:67:in `action_request_cert'
[-] 3.19.108.211:445 -   /home/tmoose/rapid7/metasploit-framework/modules/auxiliary/admin/dcerpc/icpr_cert.rb:53:in `run'
[*] Auxiliary module execution completed
msf6 auxiliary(admin/dcerpc/icpr_cert) >
  1. This fixes query_ldap_server to return an empty array rather than nil in ldap_esc_vulnerable_cert_finder
    
    [-] Auxiliary failed: NoMethodError undefined method `empty?' for nil:NilClass
    [-] Call stack:
    [-]   /home/tmoose/rapid7/metasploit-framework/modules/auxiliary/gather/ldap_esc_vulnerable_cert_finder.rb:297:in `find_esc13_vuln_cert_templates'
    [-]   /home/tmoose/rapid7/metasploit-framework/modules/auxiliary/gather/ldap_esc_vulnerable_cert_finder.rb:474:in `block in run'
    [-]   /home/tmoose/.rvm/gems/ruby-3.0.2/gems/net-ldap-0.18.0/lib/net/ldap.rb:644:in `block in open'
    [-]   /home/tmoose/.rvm/gems/ruby-3.0.2/gems/net-ldap-0.18.0/lib/net/ldap.rb:716:in `block in open'
    [-]   /home/tmoose/.rvm/gems/ruby-3.0.2/gems/net-ldap-0.18.0/lib/net/ldap/instrumentation.rb:19:in `instrument'
    [-]   /home/tmoose/.rvm/gems/ruby-3.0.2/gems/net-ldap-0.18.0/lib/net/ldap.rb:711:in `open'
    [-]   /home/tmoose/.rvm/gems/ruby-3.0.2/gems/net-ldap-0.18.0/lib/net/ldap.rb:644:in `open'
    [-]   /home/tmoose/rapid7/metasploit-framework/lib/msf/core/exploit/remote/ldap.rb:114:in `ldap_open'
    [-]   /home/tmoose/rapid7/metasploit-framework/lib/msf/core/exploit/remote/ldap.rb:98:in `ldap_connect'
    [-]   /home/tmoose/rapid7/metasploit-framework/modules/auxiliary/gather/ldap_esc_vulnerable_cert_finder.rb:457:in `run'
    [*] Auxiliary module execution completed

3. This fixes an issue in `gather/ldap_esc_vulnerable_cert_finder` because the module did not require a password value, but when the `nil` password was sent on to `ruby_smb`, it crashed:

[-] Auxiliary failed: NoMethodError undefined method force_encoding' for nil:NilClass [-] Call stack: [-] /home/tmoose/.rvm/gems/ruby-3.0.2/gems/ruby_smb-3.3.5/lib/ruby_smb/ntlm/custom/string_encoder.rb:14:inencode_utf16le' [-] /home/tmoose/.rvm/gems/ruby-3.0.2/gems/rubyntlm-0.6.3/lib/net/ntlm/client/session.rb:187:in oem_or_unicode_str' [-] /home/tmoose/.rvm/gems/ruby-3.0.2/gems/rubyntlm-0.6.3/lib/net/ntlm/client/session.rb:172:inpassword' [-] /home/tmoose/.rvm/gems/ruby-3.0.2/gems/ruby_smb-3.3.5/lib/ruby_smb/ntlm/client.rb:55:in ntlmv2_hash' [-] /home/tmoose/.rvm/gems/ruby-3.0.2/gems/ruby_smb-3.3.5/lib/ruby_smb/ntlm/client.rb:63:incalculate_user_session_key!' [-] /home/tmoose/.rvm/gems/ruby-3.0.2/gems/ruby_smb-3.3.5/lib/ruby_smb/ntlm/client.rb:20:in authenticate!' [-] /home/tmoose/.rvm/gems/ruby-3.0.2/gems/ruby_smb-3.3.5/lib/ruby_smb/ntlm/client.rb:74:ininit_context' [-] /home/tmoose/rapid7/metasploit-framework/lib/metasploit/framework/ldap/client.rb:104:in block in ldap_auth_opts_ntlm' [-] /home/tmoose/.rvm/gems/ruby-3.0.2/gems/net-ldap-0.18.0/lib/net/ldap/auth_adapter/sasl.rb:54:inblock in bind' [-] /home/tmoose/.rvm/gems/ruby-3.0.2/gems/net-ldap-0.18.0/lib/net/ldap/auth_adapter/sasl.rb:38:in loop' [-] /home/tmoose/.rvm/gems/ruby-3.0.2/gems/net-ldap-0.18.0/lib/net/ldap/auth_adapter/sasl.rb:38:inbind' [-] /home/tmoose/.rvm/gems/ruby-3.0.2/gems/net-ldap-0.18.0/lib/net/ldap/connection.rb:281:in block in bind' [-] /home/tmoose/.rvm/gems/ruby-3.0.2/gems/net-ldap-0.18.0/lib/net/ldap/instrumentation.rb:19:ininstrument' [-] /home/tmoose/.rvm/gems/ruby-3.0.2/gems/net-ldap-0.18.0/lib/net/ldap/connection.rb:278:in bind' [-] /home/tmoose/.rvm/gems/ruby-3.0.2/gems/net-ldap-0.18.0/lib/net/ldap.rb:715:inblock in open' [-] /home/tmoose/.rvm/gems/ruby-3.0.2/gems/net-ldap-0.18.0/lib/net/ldap/instrumentation.rb:19:in instrument' [-] /home/tmoose/.rvm/gems/ruby-3.0.2/gems/net-ldap-0.18.0/lib/net/ldap.rb:711:inopen' [-] /home/tmoose/.rvm/gems/ruby-3.0.2/gems/net-ldap-0.18.0/lib/net/ldap.rb:644:in open' [-] /home/tmoose/rapid7/metasploit-framework/lib/msf/core/exploit/remote/ldap.rb:114:inldap_open' [-] /home/tmoose/rapid7/metasploit-framework/lib/msf/core/exploit/remote/ldap.rb:98:in ldap_connect' [-] /home/tmoose/rapid7/metasploit-framework/modules/auxiliary/gather/ldap_esc_vulnerable_cert_finder.rb:455:inrun' [*] Auxiliary module execution completed msf6 auxiliary(gather/ldap_esc_vulnerable_cert_finder) >

bwatters-r7 commented 3 weeks ago

Release Notes

This PR fixes several instances where we we pass nil values rather than the types expected, causing crashes and stack traces in LDAP-related modules.