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 RPORT tab completion crash when connected to remote dataservice #15194

Merged
merged 1 commit into from
Jun 1, 2021

Conversation

agalway-r7
Copy link
Contributor

@agalway-r7 agalway-r7 commented May 13, 2021

Fixes #15075

Removes usage of has_host?from the option_values_target_ports module of module_option_tab_completion.

This is the last remaining use of the method in the code base, so I deleted the original method definition since it's breaking things and we don't want it being used in the future.

Verification

List the steps needed to make sure this thing works

  • Start msfconsole
  • use auxiliary/scanner/ssh/ssh_login
  • set RHOSTS 1.1.1.1
  • set RPORT <TAB>
  • Ensure that no error is thrown, and that a RPORT number is populated if appropriate

@agalway-r7 agalway-r7 marked this pull request as draft May 13, 2021 14:07
@adfoster-r7
Copy link
Contributor

Looks like this needs to handle the msfconsole crashing on tab completion still 👀

Comment on lines -216 to -218
if res.empty?
res << rand(1..65534).to_s
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can anyone explain why we were returning a random port suggestion when we couldn't find one in the DB?

@agalway-r7 agalway-r7 marked this pull request as ready for review May 17, 2021 14:40
@gwillcox-r7 gwillcox-r7 self-assigned this May 17, 2021
@gwillcox-r7
Copy link
Contributor

Can confirm that with new changes this fixes the tab completion error. master was still crashing with the error shown in #15075, but testing on this PR had the following result:

msf6 > use auxiliary/scanner/ssh/ssh_login
msf6 auxiliary(scanner/ssh/ssh_login) > set RHOSTS 1.1.1.1
RHOSTS => 1.1.1.1
msf6 auxiliary(scanner/ssh/ssh_login) > set RPORT 22 

@gwillcox-r7 gwillcox-r7 removed their assignment May 17, 2021
@rl_saved_proc = tab_complete_proc
::Readline.completion_proc = wrap_tab_compete_proc(tab_complete_proc)

@rl_saved_proc = wrap_tab_compete_proc(tab_complete_proc)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this wraps the previous readline completion proc in a try/catch, not sure if that's what we want to happen here? 👀

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it's wrapping the tab_complete_proc that gets passed in with initialize. The previous tab_complete_proc is getting overwritten here

@adfoster-r7
Copy link
Contributor

adfoster-r7 commented Jun 1, 2021

Tested with:

msfconsole
db_nmap 127.0.0.1

use auxiliary/scanner/ssh/ssh_login
set RHOSTS 127.0.0.1
set RPORT <TAB>

set RHOSTS 1.1.1.1
set RPOT <tab>

@adfoster-r7 adfoster-r7 changed the title removes obsolete has_host? usage Fix RPORT tab completion crash when connected to remote dataservice Jun 1, 2021
@adfoster-r7
Copy link
Contributor

@msjenkins-r7 retest this please

@adfoster-r7 adfoster-r7 merged commit 11fb9e8 into rapid7:master Jun 1, 2021
@adfoster-r7
Copy link
Contributor

adfoster-r7 commented Jun 1, 2021

Release Notes

Fixed a bug where msfconsole would crash when connected to a remote dataservice and tab completing possible RPORT values.

@pbarry-r7 pbarry-r7 added the bug label Jun 3, 2021
@pbarry-r7 pbarry-r7 added the rn-fix release notes fix label Jun 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug rn-fix release notes fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash When Tab Completing RPORT
4 participants