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 the default meterpreter prompt #12482

Merged
merged 1 commit into from Oct 25, 2019

Conversation

@zeroSteiner
Copy link
Member

zeroSteiner commented Oct 23, 2019

Alright this aims to fix #12481. It completely overhauls the parsing logic for a couple of benefits. So the underlying issue is that the color codes are %[a-zA-Z]{3,5} while the prompt vars are %[a-zA-Z] and there isn't a way that I'm aware of to create a proper lexer for parsing the stream of characters because there is no delimiter. Ideally we'd do something like %\{\w+\} but oh well.

The proposed solution is to process the larger color codes first, this unfortunately means that the Rex::Text::Color.subtitute_colors function is called twice. Once before the variables substitution and a second time (in the original location) by Rex::Ui::Text::Output.update_prompt. This at least means that the larger more specific codes are processed before the smaller variables are. Specifically %und will be processed and replaced before %u is, fixing the original issue that folks were identifying.

I also changed the logic to remove the possibility of a variable introducing another variable. For example in meterpreter, if the current working directory (%D) were displayed and you were in a folder named Test%H, the prompt would also substitute the hostname into the directory because %D is processed before %H.

Verification

  • Start msfconsole
  • Use the original / default MeterpreterPrompt value
  • Open a meterpreter session
  • See the proper meterpreter prompt, without your username in it

Substitution Error Example

This is an additional issue fixed by this PR

msf > set MeterpreterPrompt "%U@%H %D"
MeterpreterPrompt => %U@%H %D
msf > sessions -i -1
[*] Starting interaction with 1...

steiner@localhost.localdomain /tmp/localhost.localdomain > pwd
/tmp/%H
steiner@localhost.localdomain /tmp/localhost.localdomain > 
@acammack-r7

This comment has been minimized.

Copy link
Contributor

acammack-r7 commented Oct 23, 2019

It seems to me that a simpler solution would be to avoid having ambiguous substitutions like the original console prompt substitution. The characters to avoid are: b, c, d, g, m, r, u, w, and y. As for avoiding double substitution, I would recommend using https://ruby-doc.org/stdlib-2.6.4/libdoc/strscan/rdoc/StringScanner.html to scan through the string like:

require 'strscan'

formatted = ''
s = StringScanner.new(prompt)
while rest?
  if m = s.scan_until /%(\w)/
    if substitutable_chars.include? s[1]
      formatted << m.sub(s[0], '')
      formatted << substitution_for(s[1], framework, session)
    else
      formatted << m
    end
  else
    # No more substitutions
    formatted << s.rest
    break
  end
end
@zeroSteiner

This comment has been minimized.

Copy link
Member Author

zeroSteiner commented Oct 23, 2019

Avoiding the ambiguous characters would remove the logical relationship of the remote end being in uppercase while the local end is in lowercase which I think is a valuable tool for the end user to remember them.

Using string scanner seems simple enough and like the logic wouldn't change dramatically. I should be able to update the PR to use that tonight instead of Enumerable.each_cons.

@busterb

This comment has been minimized.

Copy link
Member

busterb commented Oct 25, 2019

We could go ahead and land this to fix the regression, and iterate in another PR later perhaps.

@busterb

This comment has been minimized.

Copy link
Member

busterb commented Oct 25, 2019

msf5 exploit(multi/handler) > setg MeterpreterPrompt %undmeterpreter%clr
MeterpreterPrompt => meterpreter
msf5 exploit(multi/handler) > run

[!] You are binding to a loopback address by setting LHOST to 127.0.0.1. Did you want ReverseListenerBindAddress?
[*] Started reverse TCP handler on 127.0.0.1:4444 
[*] Sending stage (53754 bytes) to 127.0.0.1
[*] Meterpreter session 1 opened (127.0.0.1:4444 -> 127.0.0.1:51152) at 2019-10-25 11:23:42 -0400

meterpreter >
@busterb busterb self-assigned this Oct 25, 2019
bcook-r7 pushed a commit that referenced this pull request Oct 25, 2019
@bcook-r7 bcook-r7 merged commit e771147 into rapid7:master Oct 25, 2019
3 checks passed
3 checks passed
Metasploit Automation - Sanity Test Execution Successfully completed all tests.
Details
Metasploit Automation - Test Execution Successfully completed all tests.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
msjenkins-r7 added a commit that referenced this pull request Oct 25, 2019
@busterb

This comment has been minimized.

Copy link
Member

busterb commented Oct 25, 2019

Release Notes

This fixes a regression in displaying the default Meterpreter prompt where the username of the Meterpreter session was prepended to the prompt due to misordered character substitution.

@tperry-r7 tperry-r7 added the rn-fix label Oct 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.