From 5ba02aebe88e231ead02e4a4de4cbdfa6cb05299 Mon Sep 17 00:00:00 2001
From: Danil Somsikov <dsv@chromium.org>
Date: Tue, 10 Sep 2024 12:22:30 +0000
Subject: [PATCH] [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 <dsv@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/597949
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
---
 .../src/front_end/panels/network/NetworkLogView.ts         | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/chromium/third_party/devtools-frontend/src/front_end/panels/network/NetworkLogView.ts b/chromium/third_party/devtools-frontend/src/front_end/panels/network/NetworkLogView.ts
index f2cc8ca2ef3a..73d6e02d1fe7 100644
--- a/chromium/third_party/devtools-frontend/src/front_end/panels/network/NetworkLogView.ts
+++ b/chromium/third_party/devtools-frontend/src/front_end/panels/network/NetworkLogView.ts
@@ -2184,8 +2184,7 @@ export class NetworkLogView extends Common.ObjectWrapper.eventMixin<EventTypes,
     const ignoredHeaders = new Set<string>(['accept-encoding', 'host', 'method', 'path', 'scheme', 'version']);
 
     function escapeStringWin(str: string): string {
-      /* If there are no new line characters do not escape the " characters
-         since it only uglifies the command.
+      /* Always escape the " characters so that we can use caret escaping.
 
          Because cmd.exe parser and MS Crt arguments parsers use some of the
          same escape characters, they can interact with each other in
@@ -2211,11 +2210,11 @@ export class NetworkLogView extends Common.ObjectWrapper.eventMixin<EventTypes,
          new line is there to enact the escape command the second is the character
          to escape (in this case new line).
         */
-      const encapsChars = /[\r\n]/.test(str) ? '^"' : '"';
+      const encapsChars = '^"';
       return encapsChars +
           str.replace(/\\/g, '\\\\')
               .replace(/"/g, '\\"')
-              .replace(/[^a-zA-Z0-9\s_\-:=+~'\/.',?;()*`&]/g, '^$&')
+              .replace(/[^a-zA-Z0-9\s_\-:=+~'\/.',?;()*`]/g, '^$&')
               .replace(/%(?=[a-zA-Z0-9_])/g, '%^')
               .replace(/\r?\n/g, '^\n\n') +
           encapsChars;