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 bug in tab completion of directories #16266

Merged
merged 1 commit into from Mar 3, 2022

Conversation

smashery
Copy link
Contributor

@smashery smashery commented Mar 3, 2022

This fixes several bugs in local directory tab completion.

The code currently just performs a glob based on the user's current input. This has a bug that effectively prevents it from ever working. It performs a string concatenation to add an asterisk, which alters the str parameter with the in-place-modification method concat. The str parameter is used by the readline library to do its tab completion magic, so suddenly it's got an asterisk on there, and readline's string matching fails. As a practical example:

  • The current directory contains the directories 'tools' and 'test'
  • We type cd t and press <tab>
  • The glob adds an asterisk to str to give it the value t*
  • The glob finds both the directories and returns them
  • Readline receives these two values, but also uses the modified t* value, because of the in-place modification that occurred as a result of using concat
  • Because neither tools nor test start with the literal string t*, it finds no matches, and provides no tab completion to the user.

Steps to reproduce:

  • Launch msfconsole from a directory containing at least two folders
  • Type cd and press <tab>
  • You'll see it pop up ./
  • Press <tab> again
  • Nothing happens

Expected behaviour:

The tab completion provides options based on the current directory, or absolute directory specified.

Fix:

A simple fix would be to change str.concat(*) to str + '*'. However, this leaves another subtle bug in that it doesn't respect glob-meaningful characters such as *, ?, [ and ]. So if folders actually contain these characters, it would not provide the expected results (as the user is not aware that globbing is used). ? and * work by coincidence, as globbing will use them as wildcards, which will match the literal characters ? and *; but it fails in the case of folder names containing brackets. This is probably a rarer situation, but still a bug.

You could do all sorts of glob backslash escaping, but that starts to get messy, so instead I just implemented directory searching so as to control this behaviour ourselves, in order to keep it Readline-friendly.

After the fix:

msf6 > cd co<tab>
cd config/    cd coverage/  
msf6 > cd /t<tab>
cd /testing/  cd /tmp/      cd /tools/
msf6 > cd /tmp/globtest/a[<tab>
cd /tmp/globtest/a[hi]b/     cd /tmp/globtest/a[there]b/

@smashery smashery added the bug label Mar 3, 2022
Comment on lines +305 to +306
dirs = dirs.select { |x| File.directory?(x) }
dirs = dirs.select { |x| File.directory?(x) }
Copy link
Contributor

Choose a reason for hiding this comment

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

This line has been duplicated.

This line has been duplicated.

@smcintyre-r7 smcintyre-r7 self-assigned this Mar 3, 2022
@smcintyre-r7
Copy link
Contributor

Tested and confirmed that I could reproduce the original issue and that the proposed changes fixes it. Tab completion is always hard to demo but the screenshot is a broadcast group using terminator with the unpatched version on the left with the fixed one on the right.

image

I'm going to resolve the comment about the duplicate line (I tested it with that removed) and get this landed. Thanks @smashery !

@smcintyre-r7 smcintyre-r7 added the rn-fix release notes fix label Mar 3, 2022
smcintyre-r7 added a commit that referenced this pull request Mar 3, 2022
@smcintyre-r7 smcintyre-r7 merged commit f6e88d0 into rapid7:master Mar 3, 2022
@smcintyre-r7
Copy link
Contributor

Release Notes

This fixes a bug in how msfconsole tab completes directory paths.

@smashery smashery deleted the dir_tab_complete_bugfix branch November 24, 2023 06:01
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.

None yet

3 participants