Skip to content

Commit 5ba02ae

Browse files
danilsomsikovmibrunin
authored andcommitted
[Backport] CVE-2024-9965: Insufficient data validation in DevTools
Manual backport of patch originally reviewed on https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5850610: Fix curl escaping for cmd when ampersand follows a newline. Currently, generated command allows arbitrary code execution, due to incomplete escaping: an ampersand in multi-line strings is not escaped. This is fixed by always escaping outer quotes, thus disabling cmd double quote escaping, and escaping ampersand with a caret. Double quote escaping is very convenient and easy to read, but it doesn't support multi-line strings. Thus we need to be able to escape individual characters with caret, which was added in https://codereview.chromium.org/2182213006/. That CL only escaped outer double quotes around multi-line strings for better readability. However this lead to carets being interpreted as a literal character in single-line strings. This caused crrev.com/2514441 which disabled caret escaping of ampersand even for multi-line strings with escaped quotes and crrev.com/c/5126051 which enabled escaping quotes for the single-line strings with special characters (including ampersand, which was still exempt from caret escaping:). As this is all rather complicated and hard to follow, this CL sacrifices the generated command readability in favor of correctness and simplicity (relatively speaking) of the generator code, leaving only one mode of escaping. Bug: 352651673 Change-Id: I4c25b165c6ba7b3eae3891179c8e371fc16c91f2 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5850610 Commit-Queue: Danil Somsikov <[email protected]> Reviewed-by: Benedikt Meurer <[email protected]> Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/597949 Reviewed-by: Michal Klocek <[email protected]>
1 parent 41183a7 commit 5ba02ae

File tree

1 file changed

+3
-4
lines changed

1 file changed

+3
-4
lines changed

chromium/third_party/devtools-frontend/src/front_end/panels/network/NetworkLogView.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2184,8 +2184,7 @@ export class NetworkLogView extends Common.ObjectWrapper.eventMixin<EventTypes,
21842184
const ignoredHeaders = new Set<string>(['accept-encoding', 'host', 'method', 'path', 'scheme', 'version']);
21852185

21862186
function escapeStringWin(str: string): string {
2187-
/* If there are no new line characters do not escape the " characters
2188-
since it only uglifies the command.
2187+
/* Always escape the " characters so that we can use caret escaping.
21892188
21902189
Because cmd.exe parser and MS Crt arguments parsers use some of the
21912190
same escape characters, they can interact with each other in
@@ -2211,11 +2210,11 @@ export class NetworkLogView extends Common.ObjectWrapper.eventMixin<EventTypes,
22112210
new line is there to enact the escape command the second is the character
22122211
to escape (in this case new line).
22132212
*/
2214-
const encapsChars = /[\r\n]/.test(str) ? '^"' : '"';
2213+
const encapsChars = '^"';
22152214
return encapsChars +
22162215
str.replace(/\\/g, '\\\\')
22172216
.replace(/"/g, '\\"')
2218-
.replace(/[^a-zA-Z0-9\s_\-:=+~'\/.',?;()*`&]/g, '^$&')
2217+
.replace(/[^a-zA-Z0-9\s_\-:=+~'\/.',?;()*`]/g, '^$&')
22192218
.replace(/%(?=[a-zA-Z0-9_])/g, '%^')
22202219
.replace(/\r?\n/g, '^\n\n') +
22212220
encapsChars;

0 commit comments

Comments
 (0)