Closed Bug 1777800 (CVE-2023-23599) Opened 2 years ago Closed 1 year ago

devtools "Copy as cURL (Windows)" allows custom code execution in CMD

Categories

(DevTools :: Netmonitor, defect, P2)

defect

Tracking

(firefox-esr102109+ fixed, firefox108 wontfix, firefox109+ fixed, firefox110+ fixed)

RESOLVED FIXED
110 Branch
Tracking Status
firefox-esr102 109+ fixed
firefox108 --- wontfix
firefox109 + fixed
firefox110 + fixed

People

(Reporter: vadcx, Assigned: bomsy)

References

Details

(Keywords: csectype-priv-escalation, sec-moderate, Whiteboard: [reporter-external] [client-bounty-form] [verif?][post-critsmash-triage][adv-main109+][adv-esr102.7+])

Attachments

(8 files, 1 obsolete file)

It is possible to construct a request which, when copied as a cURL command on Windows, will automatically execute arbitrary commands when pasted in cmd.exe.
Affected: devtools "Copy as cURL (Windows)" -> cmd.exe (not Powershell)
Version: Firefox 102 and older

For example, the following request body will, upon pasting in cmd.exe:

  1. Automatically execute curl with query evil
  2. Auto start cmd.exe with custom parameters
  3. Auto start calc.exe

query=evil\r\rcmd" /c timeout /t 3 & calc.exe\r\r

This turns into:
curl command ... --data-raw "query=evil"^
More?
More?
<executes curl>
"cmd"" /c timeout /t 3 & calc.exe"^
More?
More?
<executes attacker's cmd command>


History:

Exists since April 2019, current code: https://hg.mozilla.org/mozilla-central/rev/c3f507baa3dea6e86274de6d57d3e638f2b3bb47#l2.89
This attempt tried to fix the older code, but didn't account for single \r characters.


Background:
The \r\n new-line is the "native" new-line form on Windows, but CMD seems to have its own opinion on that.

Clipboard API: passed as-is, \r\n and \r and \n can roundtrip.
Writing to a file in text mode: \r\n turns into \r\r\n, \n -> \r\n, \r -> \r

Now comes the CMD:
\r -> as if <ENTER> is pressed
\r\n -> like <ENTER>
\n -> Win10: no character at all
\n -> Win7: like <ENTER>

I repeated this in the .html file descriptions below.


PoC descriptions:

Win7-LFLF.html: Only automatically executes on Windows 7's CMD. With Win10 CMD the newline characters are lost
-> Automatic command execution when pasted

Win10-LFLF.html: Due to newline chars being gobbled, the ^ escape now escapes a double-quote. We can abuse this in a different way to work on Win10.
-> User must press ENTER to execute our command

Windows-CRCR.html: Due to Windows 10 CMD bug/feature of \n handling, this is the only variant that works on older and newer systems at the same time.
-> Automatic command execution when pasted


Suggested fix:

  1. Split "Copy as cURL (Windows)" in two separate commands: for CMD and for Powershell. Their parsers are completely different. While I couldn't come up with a variant that'd work for PS, the current escapeStringWin() code produces garbage data for either shell when a string is supposed to be escaped.

  2. A separate Powershell escape implementation

  3. CMD escape impl: Only " \ % \r \n must be escaped, when using the current double-quote method.
    3.1: Step 1: Escape double-quotes "", percent signs "%". Escape backslashes when followed by double quotes "
    3.2: Step 2: Normalize all new-line characters (they are inevitably (and already) mangled when using CMD)
    3.2.1: replace \r\n -> \n
    3.2.2: replace \r -> \n
    3.2.3: replace \n -> \r\n
    3.2.4: escape new-lines

Note on 3.2.3: When testing ^\r\n\r\n new-lines through console.log, while displayed correctly, new-lines ended up duplicated when copied to clipboard. The ^\n\n construction displays and pastes correctly from FF console. However I think devtools' copy code doesn't modify the data, so there it must still be ^\r\n\r\n. Please test.

Further reading:
https://daviddeley.com/autohotkey/parameters/parameters.htm
http://www.windowsinspired.com/how-a-windows-programs-splits-its-command-line-into-individual-arguments/
http://www.windowsinspired.com/understanding-the-command-line-string-and-arguments-received-by-a-windows-program/
http://www.windowsinspired.com/summary-of-important-concepts-for-quoting-and-escaping-command-line-arguments/


escapeStringWinCmd

While describing the escape behaviour above, I ended up implementing it myself:

function escapeStringWinCmd(str) {
return (
'"' +
str
.replaceAll('"', '""')
.replaceAll("%", '"%"')
.replace(/([\]+)(")/g, "$1$1$2") // escape backslashes only when followed by "
.replaceAll("\r\n", '\n')
.replaceAll("\r", '\n')
.replaceAll("\n", '"^\r\n\r\n"') +
'"'
);
}
// The testStr must paste and execute cleanly after escaping:
// curl --trace trace.txt https://example.com/404 --data-raw <here>

// testStr = 'Lets CRLF \r\n then LF \n then CR \r then doublequote" then singlequote ' then percent % then env %os% then ampersand & then backslash \ 2bs \\ 3bs \\\ 1bsQ \" 2bsQ \\" 3bsQ \\\" then special chars ~!@#$%^&*()_+=-0987654321`,./<?>}]{[ |\/ end'


Testing

To test this, use curl --trace log.txt option, to see what curl actually receives on the command-line. That's because each Windows program does its own parsing for argv.

Example in CMD on Win10 (press ENTER for "More?"):
curl --trace trace.txt http://example.com/404 --data-raw "doublequote"" and percent "%" and backslash \ and new"^
More?
More? "line"

Here the new line is passed as "new\nline" to the server.

=> Send data, 56 bytes (0x38)
0000: 64 6f 75 62 6c 65 71 75 6f 74 65 22 20 61 6e 64 doublequote" and
0010: 20 70 65 72 63 65 6e 74 20 25 20 61 6e 64 20 62 percent % and b
0020: 61 63 6b 73 6c 61 73 68 20 5c 5c 20 61 6e 64 20 ackslash \ and
0030: 6e 65 77 0a 6c 69 6e 65 new.line

Backslash escape:
curl --trace trace.txt http://example.com/404 --data-raw "backslash escape \"

=> Send data, 18 bytes (0x12)
0000: 62 61 63 6b 73 6c 61 73 68 20 65 73 63 61 70 65 backslash escape
0010: 20 5c \

Other special cases:
curl --trace trace.txt http://example.com/404 --data-raw "doublequote"" and percent "%" and backslash \ double slash \ and triple slash \\ and bsq \"" and dbsq \\"" and tbsq\\\"" end"

=> Send data, 120 bytes (0x78)
0000: 64 6f 75 62 6c 65 71 75 6f 74 65 22 20 61 6e 64 doublequote" and
0010: 20 70 65 72 63 65 6e 74 20 25 20 61 6e 64 20 62 percent % and b
0020: 61 63 6b 73 6c 61 73 68 20 5c 20 64 6f 75 62 6c ackslash \ doubl
0030: 65 20 73 6c 61 73 68 20 5c 5c 20 61 6e 64 20 74 e slash \ and t
0040: 72 69 70 6c 65 20 73 6c 61 73 68 20 5c 5c 5c 20 riple slash \\
0050: 61 6e 64 20 62 73 71 20 5c 22 20 61 6e 64 20 64 and bsq " and d
0060: 62 73 71 20 5c 5c 22 20 61 6e 64 20 74 62 73 71 bsq \" and tbsq
0070: 5c 5c 5c 22 20 65 6e 64 \" end

Flags: sec-bounty?
Attached file Win7-LFLF-CMD.html
Attached file Win10-LFLF-CMD.html
Attached file Windows-CRCR-CMD.html

My description in "History" is incorrect: https://searchfox.org/mozilla-central/rev/da3cc1b31be7a7d919d6ebde4e3c033a83446e05/devtools/client/shared/curl.js#465
What ends up happening: \r\r are turned into ^\r\r\r\r, thus duplicating the amount of newlines as perceived by cmd.

Component: Security → Netmonitor
Product: Firefox → DevTools

The severity field is not set for this bug.
:Honza, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(odvarko)
Severity: -- → S2
Flags: needinfo?(odvarko)
Priority: -- → P2
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: nobody → hmanilla
Status: NEW → ASSIGNED
Depends on: 1795595

Comment on attachment 9295386 [details]
Bug 1777800 - [devtools] Add 'Copy as PowerShell' command to request list context menu

Revision D157711 was moved to bug 1795595. Setting attachment 9295386 [details] to obsolete.

Attachment #9295386 - Attachment is obsolete: true
Attachment #9295387 - Attachment description: Bug 1777800 - [devtools] Update the curl escape string for windows to no longer try to handle powershell → Bug 1777800 - [devtools] Update the curl escape string for windows to no longer try to handle powershell r=ochameau

Hi Honza,
Could you help test this example I've tried to put together on windows with the PATCH attached to the bug? https://likeable-full-grease.glitch.me/

Thanks

Flags: needinfo?(odvarko)
Attached file test-curl-escapes.txt

I followed the instructions and executed 4 commands, results of 4 "Copy as cURL (Windows)".
Consequently I copied the entire cmd.exe output into an attached file.
Happy to provide more info and/or repeat the test

Note that my OS is in Czech, so one translation:
"Systém nemůže nalézt uvedenou cestu." => "The system can't find the path"

Flags: needinfo?(odvarko)

Thanks alot Honza! This is super helpful. i'll ping you for a retest.

Hi Honza,
I've updated the patch.
Could you please help retest when you have a chance?

Flags: needinfo?(odvarko)

Sure thing, attaching the results from my command line.

Flags: needinfo?(odvarko)

Hey bomsy, I hope you didn't miss my example fix above for CMD: "escapeStringWinCmd". I didn't test it 100%, but judging by Honza's console output, it ought to be better :) Unfortunately I can't see where you're at currently (no access to Phabricator).
I notice there's at least one test case my "testStr" is missing, a backslash as the last string character.

Finally, if testing continues to be a massive problem, I'll try to create an automated test for all ASCII characters to go through CMD/PowerShell (outside of FF of course). As a result of that work there'll be a definitive test string + replacement algorithm. It's just not feasible to go through all edge-cases by hand.

I haven't given it much thought yet, but something gotta be possible with AutoIt scripting on Windows; so this is an offer.
~bugreporter

Hi Daniel,

(In reply to Vadim [:vadcx] from comment #14)

Hey bomsy, I hope you didn't miss my example fix above for CMD: "escapeStringWinCmd". I didn't test it 100%, but judging by Honza's console output, it ought to be better :) Unfortunately I can't see where you're at currently (no access to Phabricator).

Oh yeah i'll add that test to the list so we can explicitly test that.

Finally, if testing continues to be a massive problem, I'll try to create an automated test for all ASCII characters to go through CMD/PowerShell (outside of FF of course). As a result of that work there'll be a definitive test string + replacement algorithm. It's just not feasible to go through all edge-cases by hand.

I haven't given it much thought yet, but something gotta be possible with AutoIt scripting on Windows; so this is an offer.

I'm not familiar with autoit scripting. But also automated test script which can be run on Windows would help, so feel free to provide.

Thanks

Attachment #9295387 - Attachment description: Bug 1777800 - [devtools] Update the curl escape string for windows to no longer try to handle powershell r=ochameau → Bug 1777800 - [devtools] Update the curl escape string for windows to no longer try to handle powershell r=nchevobbe

Hi Honza,
Could you please do another test. i've included another test case from that provided by daniel.
https://likeable-full-grease.glitch.me/

Thanks

Flags: needinfo?(odvarko)

Sure thing, attaching results from my command line (cmd.exe)

Flags: needinfo?(odvarko)

[devtools] Update the curl escape string for windows to no longer try to handle powershell r=nchevobbe
https://hg.mozilla.org/integration/autoland/rev/c46a68d21e7f5b5c186ff3499729e5427d6f3944
https://hg.mozilla.org/mozilla-central/rev/c46a68d21e7f

Group: firefox-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 110 Branch
Flags: sec-bounty? → sec-bounty+

Please nominate this for Beta and ESR approval when you get a chance.

Flags: needinfo?(hmanilla)
Flags: in-testsuite+

Comment on attachment 9295387 [details]
Bug 1777800 - [devtools] Update the curl escape string for windows to no longer try to handle powershell r=nchevobbe

Beta/Release Uplift Approval Request

  • User impact if declined: Security risks for developer if this patch is declined
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Low risk. Just code to escape curl commands properly
  • String changes made/needed:
  • Is Android affected?: Unknown

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration:
  • User impact if declined: Security risks for developer if this patch is declined
  • Fix Landed on Version: 109
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Low risk. Just code to escape curl commands properly
Flags: needinfo?(hmanilla)
Attachment #9295387 - Flags: approval-mozilla-esr102?
Attachment #9295387 - Flags: approval-mozilla-beta?

Comment on attachment 9295387 [details]
Bug 1777800 - [devtools] Update the curl escape string for windows to no longer try to handle powershell r=nchevobbe

Approved for 109.0b3 and 102.7esr.

Attachment #9295387 - Flags: approval-mozilla-esr102?
Attachment #9295387 - Flags: approval-mozilla-esr102+
Attachment #9295387 - Flags: approval-mozilla-beta?
Attachment #9295387 - Flags: approval-mozilla-beta+
Flags: qe-verify-
Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [reporter-external] [client-bounty-form] [verif?][post-critsmash-triage]
Whiteboard: [reporter-external] [client-bounty-form] [verif?][post-critsmash-triage] → [reporter-external] [client-bounty-form] [verif?][post-critsmash-triage][adv-main109+][adv-esr102.7+]
Alias: CVE-2023-23599
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.