Fix session crashing when unsetting smbuser or smbpass for a smb module

[3V3RYONE]

This PR fixes #15916

Cause of bug:
The cause of this issue was that, the login_scanner.rb code checked for either valued creds or empty strings before applying force_encoding method on them. There was no check on nil values explicitly. So unsetting the smbuser and smbpass makes the creds nil values which raises the undefined method force_encoding exception.

Approach:
So one solution was to have a check on the nil values explicitly in the login_scanner.rb code. However, I figured out that login_scanner.rb always referenced to simple_client.rb while attempting a login on the creds. And in the simple_client.rb code, there was a check on the smbpass, which made it assign to an empty string if it was a nil value. Jackpot, So here I just added the same condition for smbuser as well, which solved the bug!

I chose to change the simple_client.rb because instead of applying the nil check condition on all files which have smbuser creds, it would be better to change in the sole root file which they always reference to for attempting a login.

Before:

msf6 > use exploit/windows/smb/ms07_029_msdns_zonename 
[*] No payload configured, defaulting to windows/meterpreter/reverse_tcp
msf6 exploit(windows/smb/ms07_029_msdns_zonename) > unset smbuser
Unsetting smbuser...
msf6 exploit(windows/smb/ms07_029_msdns_zonename) > unset smbpass
Unsetting smbpass...
msf6 exploit(windows/smb/ms07_029_msdns_zonename) > set RHOSTS 192.168.1.100
RHOSTS => 192.168.1.100
msf6 exploit(windows/smb/ms07_029_msdns_zonename) > run

[*] Started reverse TCP handler on 192.168.1.105:4444 
[-] 192.168.1.100:445 - Exploit failed [no-access]: Rex::Proto::SMB::Exceptions::LoginError Login Failed: undefined method `force_encoding' for nil:NilClass
[*] Exploit completed, but no session was created.  

After:

msf6 > use exploit/windows/smb/ms07_029_msdns_zonename 
[*] No payload configured, defaulting to windows/meterpreter/reverse_tcp
msf6 exploit(windows/smb/ms07_029_msdns_zonename) > unset smbuser
Unsetting smbuser...
msf6 exploit(windows/smb/ms07_029_msdns_zonename) > unset smbpass
Unsetting smbpass...
msf6 exploit(windows/smb/ms07_029_msdns_zonename) > set RHOSTS 192.168.1.100
RHOSTS => 192.168.1.100
msf6 exploit(windows/smb/ms07_029_msdns_zonename) > run

[*] Started reverse TCP handler on 192.168.1.105:4444 
[*] 192.168.1.100:445 - Unknown OS: Windows 7 Professional 7601 Service Pack 1
[*] Exploit completed, but no session was created.  

Verification

• Start msfconsole
• Use any smb module. eg - use exploit/windows/smb/ms07_029_msdns_zonename
• unset smbuser
• unset smbpass
• set RHOSTS TARGET_IP_ADDR
• run
• Verify that run completes the exploit successfully even though no session is created
• Verify that run does not crash or raise an exception.

[bcoles]

This change makes sense (and the tests pass). Tentatively approved.

I'm not sure if there are historical reasons why we do not allow blank usernames in SMB client. I believe SMB/samba requires a username.

On the other hand, comments in the auth brute library imply a blank username or blank password are permitted.

metasploit-framework/lib/msf/core/auxiliary/auth_brute.rb

Lines 549 to 552 in 56b19e5

	# Move datastore['USERNAME'] and datastore['PASSWORD'] to the front of the list.
	# Note that we cannot tell the user intention if USERNAME or PASSWORD is blank --
	# maybe (and it's often) they wanted a blank. One more credential won't kill
	# anyone, and hey, won't they be lucky if blank user/blank pass actually works!

In the test output above you tested when both smbuser and smbpass are unset. However, the force_encoding bug occurs when a password is provided but no username resulting in login failure.

What happens when:

set smbpass test
unset smbuser

[3V3RYONE]

Hey @bcoles , yep that really makes sense. Because, it may seem so that SMB would require a username somehow..

But again, going through the comments in auth library, it says that we would be lucky if blank username/password works. I guess that means we can attempt to setup an exploit even in blank usernames?

When smbpass is set but smbuser is unset, log:

msf6 exploit(windows/smb/ms07_029_msdns_zonename) > set smbpass test
smbpass => test
msf6 exploit(windows/smb/ms07_029_msdns_zonename) > unset smbuser
Unsetting smbuser...
msf6 exploit(windows/smb/ms07_029_msdns_zonename) > run

[*] Started reverse TCP handler on 192.168.1.105:4444 
[-] 192.168.1.100:445 - Exploit failed [no-access]: Rex::Proto::SMB::Exceptions::LoginError Login Failed: The server responded with error: STATUS_INVALID_PARAMETER (Command=115 WordCount=0)
[*] Exploit completed, but no session was created.  

I believe the bug #15916 was about the when both smbuser and smbpass were unset? If smbuser would be unset and smbpass would be set, it should produce the correct error message, as it is producing now right? Or should it be someway other? What do you think? Any changes to be done?

[3V3RYONE]

Any updates @bcoles ?

[cdelafuente-r7]

I'm fine with these changes too. A blank username and password should be treated as a guest or anonymous access by the underlying SMB client, mostly to access IPC$ share without authentication. This commonly known as Null Session. This was very common on older Windows with SMBv1, but not allowed anymore by default. Note that both RubySMB and Rex clients handle this correctly for SMBv1.

[3V3RYONE]

Hey @cdelafuente-r7 , thanks for the update.. The Null session makes sense..
So I guess it is can be merged?

[3V3RYONE]

Hey @bcoles @cdelafuente-r7 , can we have some updates?
I would love to make any changes as required for merging!

[smcintyre-r7]

Yeah I agree with the others that this makes sense. If user is nil, this normalizes it into a string which is what is done for pass already.

Using psexec as an example, before these changes:

msf6 exploit(windows/smb/psexec) > unset SMBUser
Unsetting SMBUser...
msf6 exploit(windows/smb/psexec) > run

[*] Started reverse TCP handler on 192.168.159.128:4444 
[*] 192.168.159.10:445 - Connecting to the server...
[*] 192.168.159.10:445 - Authenticating to 192.168.159.10:445 as user ''...
[-] 192.168.159.10:445 - Exploit failed [no-access]: Rex::Proto::SMB::Exceptions::LoginError Login Failed: undefined method `encode' for nil:NilClass
Did you mean?  encode_tlv
[*] Exploit completed, but no session was created.
msf6 exploit(windows/smb/psexec) >

After these changes:

sf6 exploit(windows/smb/psexec) > run

[*] Started reverse TCP handler on 192.168.159.128:4444 
[*] 192.168.159.10:445 - Connecting to the server...
[*] 192.168.159.10:445 - Authenticating to 192.168.159.10:445 as user ''...
[-] 192.168.159.10:445 - Exploit failed [no-access]: Rex::Proto::SMB::Exceptions::LoginError Login Failed: (0xc000006d) STATUS_LOGON_FAILURE: The attempted logon is invalid. This is either due to a bad username or authentication information.
[*] Exploit completed, but no session was created.
msf6 exploit(windows/smb/psexec) >

[smcintyre-r7]

Thanks @3V3RYONE for tracking this down and fixing it! 🎉

[smcintyre-r7]

Release Notes

This fixes a bug where modules using the SMB client would crash when the SMBUser datastore option had been explicitly unset.

[3V3RYONE]

Thanks @3V3RYONE for tracking this down and fixing it! tada

It was my pleasure working on it! I love the community and look forward to contribute more! :)