Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Catch exception from net/ssh/connection/session.rb:381 #16309

Merged
merged 3 commits into from
Mar 17, 2022

Conversation

HynekPetrak
Copy link
Contributor

Ruby gem's net-ssh-6.1.0/lib/net/ssh/connection/session.rb is throwing an exception on line 381:

        def exec(command, status: nil, &block)
          open_channel do |channel|
            channel.exec(command) do |ch, success|
              raise "could not execute command: #{command.inspect}" unless success

which is not being handled in lib/metasploit/framework/ssh/platform.rb causing the ssh_login scanner to crash on first host, where id command execution fails.

Trace I'm getting:

[*] Scanned 39 of 40 hosts (97% complete)
[-] Auxiliary failed: RuntimeError could not execute command: "id\n"
[-] Call stack:
[-]   /usr/share/metasploit-framework/vendor/bundle/ruby/2.7.0/gems/net-ssh-6.1.0/lib/net/ssh/connection/session.rb:381:in `block (2 levels) in exec'
[-]   /usr/share/metasploit-framework/vendor/bundle/ruby/2.7.0/gems/net-ssh-6.1.0/lib/net/ssh/connection/channel.rb:618:in `do_failure'
[-]   /usr/share/metasploit-framework/vendor/bundle/ruby/2.7.0/gems/net-ssh-6.1.0/lib/net/ssh/connection/session.rb:695:in `channel_failure'
[-]   /usr/share/metasploit-framework/vendor/bundle/ruby/2.7.0/gems/net-ssh-6.1.0/lib/net/ssh/connection/session.rb:548:in `dispatch_incoming_packets'
[-]   /usr/share/metasploit-framework/vendor/bundle/ruby/2.7.0/gems/net-ssh-6.1.0/lib/net/ssh/connection/session.rb:248:in `ev_preprocess'
[-]   /usr/share/metasploit-framework/vendor/bundle/ruby/2.7.0/gems/net-ssh-6.1.0/lib/net/ssh/connection/event_loop.rb:100:in `each'
[-]   /usr/share/metasploit-framework/vendor/bundle/ruby/2.7.0/gems/net-ssh-6.1.0/lib/net/ssh/connection/event_loop.rb:100:in `ev_preprocess'
[-]   /usr/share/metasploit-framework/vendor/bundle/ruby/2.7.0/gems/net-ssh-6.1.0/lib/net/ssh/connection/event_loop.rb:28:in `process'
[-]   /usr/share/metasploit-framework/vendor/bundle/ruby/2.7.0/gems/net-ssh-6.1.0/lib/net/ssh/connection/session.rb:227:in `process'
[-]   /usr/share/metasploit-framework/vendor/bundle/ruby/2.7.0/gems/net-ssh-6.1.0/lib/net/ssh/connection/session.rb:180:in `block in loop'
[-]   /usr/share/metasploit-framework/vendor/bundle/ruby/2.7.0/gems/net-ssh-6.1.0/lib/net/ssh/connection/session.rb:180:in `loop'
[-]   /usr/share/metasploit-framework/vendor/bundle/ruby/2.7.0/gems/net-ssh-6.1.0/lib/net/ssh/connection/session.rb:180:in `loop'
[-]   /usr/share/metasploit-framework/vendor/bundle/ruby/2.7.0/gems/net-ssh-6.1.0/lib/net/ssh/connection/channel.rb:272:in `wait'
[-]   /usr/share/metasploit-framework/vendor/bundle/ruby/2.7.0/gems/net-ssh-6.1.0/lib/net/ssh/connection/session.rb:427:in `exec!'
[-]   /usr/share/metasploit-framework/lib/metasploit/framework/ssh/platform.rb:14:in `block in get_platform_info'
[-]   /usr/lib/ruby/2.7.0/timeout.rb:95:in `block in timeout'
[-]   /usr/lib/ruby/2.7.0/timeout.rb:33:in `block in catch'
[-]   /usr/lib/ruby/2.7.0/timeout.rb:33:in `catch'
[-]   /usr/lib/ruby/2.7.0/timeout.rb:33:in `catch'
[-]   /usr/lib/ruby/2.7.0/timeout.rb:110:in `timeout'
[-]   /usr/share/metasploit-framework/lib/metasploit/framework/ssh/platform.rb:13:in `get_platform_info'
[-]   /usr/share/metasploit-framework/lib/metasploit/framework/login_scanner/ssh.rb:117:in `gather_proof'
[-]   /usr/share/metasploit-framework/lib/metasploit/framework/login_scanner/ssh.rb:97:in `attempt_login'
[-]   /usr/share/metasploit-framework/lib/metasploit/framework/login_scanner/base.rb:231:in `block in scan!'
[-]   /usr/share/metasploit-framework/lib/metasploit/framework/login_scanner/base.rb:179:in `block in each_credential'
[-]   /usr/share/metasploit-framework/lib/metasploit/framework/credential_collection.rb:82:in `block in each_filtered'
[-]   /usr/share/metasploit-framework/lib/metasploit/framework/credential_collection.rb:223:in `each_unfiltered'
[-]   /usr/share/metasploit-framework/lib/metasploit/framework/credential_collection.rb:79:in `each_filtered'
[-]   /usr/share/metasploit-framework/lib/metasploit/framework/login_scanner/base.rb:141:in `each_credential'
[-]   /usr/share/metasploit-framework/lib/metasploit/framework/login_scanner/base.rb:205:in `scan!'
[-]   /usr/share/metasploit-framework/modules/auxiliary/scanner/ssh/ssh_login.rb:116:in `run_host'
[-]   /usr/share/metasploit-framework/lib/msf/core/auxiliary/scanner.rb:124:in `block (2 levels) in run'
[-]   /usr/share/metasploit-framework/lib/msf/core/thread_manager.rb:105:in `block in spawn'
[-]   /usr/share/metasploit-framework/vendor/bundle/ruby/2.7.0/gems/logging-2.3.0/lib/logging/diagnostic_context.rb:474:in `block in create_with_logging_context'
[*] Auxiliary module execution completed

@smcintyre-r7
Copy link
Contributor

So it looks the the approach you opted to take would place the exception information into the info string which is then used as the platform. This seems less than ideal for 2 reasons:

  1. Now the caller can't tell that the function failed to properly identify the platform
  2. The error message is treated as the platform information, it could be stored in the database which will undoubtedly lead to confusion

Looking at the stack trace, I would instead propose that you go to where the proof is generated, add the error handling there and set it to nil on error. That will ensure that callers elsewhere in the framework can choose what to do when the platform can't be determined instead of getting a string of error information that it thinks is the platform name. In this case the login scanner would still record that the login attempt succeeded but without the proof that it was unable to gather.

@HynekPetrak
Copy link
Contributor Author

@smcintyre-r7 corrected.

HynekPetrak and others added 2 commits March 17, 2022 14:05
Co-authored-by: Spencer McIntyre <58950994+smcintyre-r7@users.noreply.github.com>
@smcintyre-r7 smcintyre-r7 merged commit ccdc2db into rapid7:master Mar 17, 2022
@smcintyre-r7
Copy link
Contributor

Looking good now. I added another instance of exception handling in acf3906 where I noticed it would fail with the same exception when trying to create the session.

@smcintyre-r7
Copy link
Contributor

Release Notes

This fixes an issue where the ssh_login module would crash when the channel used to execute the commands to gather the platform information reported that they failed.

@smcintyre-r7 smcintyre-r7 added the rn-fix release notes fix label Mar 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug library rn-fix release notes fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants