Firefox · Core
CVE-2025-5264
Logic Error in Core
Overview
Medium
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
generateCommanddevtools/client/shared/curl.js |
modified | |
ifdevtools/client/shared/curl.js |
modified |
Files Changed
devtools/client/netmonitor/test/browser_net_curl-utils.jsdevtools/client/shared/curl.jsdevtools/client/shared/test/xpcshell/test_curl.js
Patch
diff --git a/devtools/client/netmonitor/test/browser_net_curl-utils.js b/devtools/client/netmonitor/test/browser_net_curl-utils.js
index 17452405eda..da44ebfaf47 100644
--- a/devtools/client/netmonitor/test/browser_net_curl-utils.js
+++ b/devtools/client/netmonitor/test/browser_net_curl-utils.js
@@ -153,7 +153,7 @@ function testDataArgumentOnGeneratedCommand(data) {
}
function testDataEscapeOnGeneratedCommand(data) {
- const paramsWin = `--data-raw "{""param1"":""value1"",""param2"":""value2""}"`;
+ const paramsWin = `--data-raw ^"{\\"param1\\":\\"value1\\",\\"param2\\":\\"value2\\"}^"`;
const paramsPosix = `--data-raw '{"param1":"value1","param2":"value2"}'`;
let curlCommand = Curl.generateCommand(data, "WINNT");
diff --git a/devtools/client/shared/curl.js b/devtools/client/shared/curl.js
index 0af5a608cb2..35f40544045 100644
--- a/devtools/client/shared/curl.js
+++ b/devtools/client/shared/curl.js
@@ -58,17 +58,11 @@ const Curl = {
generateCommand(data, platform) {
const utils = CurlUtils;
- let command = ["curl"];
+ let commandParts = [];
// Make sure to use the following helpers to sanitize arguments before execution.
- const addParam = value => {
- const safe = /^[a-zA-Z-]+$/.test(value) ? value : escapeString(value);
- command.push(safe);
- };
-
- const addPostData = value => {
- const safe = /^[a-zA-Z-]+$/.test(value) ? value : escapeString(value);
- postData.push(safe);
+ const escapeStringifNeeded = value => {
+ return /^[a-zA-Z-]+$/.test(value) ? value : escapeString(value);
};
const ignoredHeaders = new Set();
@@ -77,17 +71,17 @@ const Curl = {
// The cURL command is expected to run on the same platform that Firefox runs
// (it may be different from the inspected page platform).
const escapeString =
- currentPlatform == "WINNT"
+ currentPlatform === "WINNT"
? utils.escapeStringWin
: utils.escapeStringPosix;
// Add URL.
- addParam(data.url);
+ commandParts.push(escapeString(data.url));
// Disable globbing if the URL contains brackets.
// cURL also globs braces but they are already percent-encoded.
if (data.url.includes("[") || data.url.includes("]")) {
- addParam("--globoff");
+ commandParts.push("--globoff");
}
let postDataText = null;
@@ -104,13 +98,13 @@ const Curl = {
// which composed using \n only, not \r\n, may be not parsable for
// peers which split parts of multipart payload using \r\n.
postDataText = data.postDataText;
- addPostData("--data-binary");
+ postData.push("--data-binary");
const boundary = utils.getMultipartBoundary(data);
const text = utils.removeBinaryDataFromMultipartText(
postDataText,
boundary
);
- addPostData(text);
+ postData.push(escapeStringifNeeded(text));
ignoredHeaders.add("content-length");
} else if (
data.postDataText &&
@@ -119,8 +113,10 @@ const Curl = {
) {
// When no postData exists, --data-raw should not be set
postDataText = data.postDataText;
- addPostData("--data-raw");
- addPostData(utils.writePostDataTextParams(postDataText));
+ postData.push(
+ "--data-raw " +
+ escapeStringifNeeded(`${utils.writePostDataTextParams(postDataText)}`)
+ );
ignoredHeaders.add("content-length");
}
// curl generates the host header itself based on the given URL
@@ -128,20 +124,19 @@ const Curl = {
// Add --compressed if the response is compressed
if (utils.isContentEncodedResponse(data)) {
- addParam("--compressed");
+ commandParts.push("--compressed");
}
// Add -I (HEAD)
// For servers that supports HEAD.
// This will fetch the header of a document only.
if (data.method === "HEAD") {
- addParam("-I");
+ commandParts.push("-I");
} else if (data.method !== "GET") {
// Add method.
// For HEAD and GET requests this is not necessary. GET is the
// default, -I implies HEAD.
- addParam("-X");
- addParam(data.method);
+ commandParts.push("-X " + escapeStringifNeeded(`${data.method}`));
}
// Add request headers.
@@ -155,14 +150,26 @@ const Curl = {
if (ignoredHeaders.has(header.name.toLowerCase())) {
continue;
}
- addParam("-H");
- addParam(header.name + ": " + header.value);
+ commandParts.push(
+ "-H " + escapeStringifNeeded(`${header.name}: ${header.value}`)
+ );
}
// Add post data.
- command = command.concat(postData);
-
- return command.join(" ");
+ commandParts = commandParts.concat(postData);
+
+ // Format with line breaks if the command has more than 2 parts
+ // e.g
+ // Command with 2 parts - curl https://foo.com
+ // Commands with more than 2 parts -
+ // curl https://foo.com
+ // -X POST
+ // -H "Accept : */*"
+ // -H "accept-language: en-US"
+ const joinStr = currentPlatform === "WINNT" ? " ^\n " : " \\\n ";
+ return (
+ "curl " + commandParts.join(commandParts.length >= 3 ? joinStr : " ")
+ );
},
};
@@ -444,18 +451,16 @@ const CurlUtils = {
same escape characters, they can interact with each other in
horrible ways, the order of operations is critical.
*/
- const encapsChars = '"';
+ const encapsChars = '^"';
return (
encapsChars +
str
-
// Replace \ with \\ first because it is an escape character for certain
// conditions in both parsers.
.replace(/\\/g, "\\\\")
- // Replace double quote chars with two double quotes (not by escaping with \") because it is
- // recognized by both cmd.exe and MS Crt arguments parser.
- .replace(/"/g, '""')
+ // Escape double quotes with double slashes.
+ .replace(/"/g, '\\"')
// Escape ` and $ so commands do not get executed e.g $(calc.exe) or `\$(calc.exe)
.replace(/[`$]/g, "\\$&")
@@ -473,15 +478,10 @@ const CurlUtils = {
// by the previous replace.
.replace(/%(?=[a-zA-Z0-9_])/g, "%^")
- // We replace \r and \r\n with \n, this allows to consistently escape all new
- // lines in the next replace
- .replace(/\r\n?/g, "\n")
-
// Lastly we replace new lines with ^ and TWO new lines because the first
// new line is there to enact the escape command the second is the character
// to escape (in this case new line).
- // The extra " enables escaping new lines with ^ within quotes in cmd.exe.
- .replace(/\n/g, '"^\r\n\r\n"') +
+ .replace(/\r?\n/g, "^\n\n") +
encapsChars
);
},
diff --git a/devtools/client/shared/test/xpcshell/test_curl.js b/devtools/client/shared/test/xpcshell/test_curl.js
index a2a6c3412ee..a04d0462fc9 100644
--- a/devtools/client/shared/test/xpcshell/test_curl.js
+++ b/devtools/client/shared/test/xpcshell/test_curl.js
@@ -230,18 +230,15 @@ add_task(async function () {
);
// Check binary data
- const dataBinaryPos = cmd.indexOf("--data-binary");
- const dataBinaryParam = `--data-binary ${isWin() ? "" : "$"}${escapeNewline(
- quote(request.postDataText)
- )}`;
+ const dataBinaryParam = `--data-binary \\\n $'------------14808\\r\\n`;
Assert.notStrictEqual(
- dataBinaryPos,
+ cmd.indexOf("--data-binary"),
-1,
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/devtools/client/netmonitor/test/browser_net_curl-utils.js b/devtools/client/netmonitor/test/browser_net_curl-utils.js
index 17452405eda..da44ebfaf47 100644
--- a/devtools/client/netmonitor/test/browser_net_curl-utils.js
+++ b/devtools/client/netmonitor/test/browser_net_curl-utils.js
@@ -153,7 +153,7 @@ function testDataArgumentOnGeneratedCommand(data) {
}
function testDataEscapeOnGeneratedCommand(data) {
- const paramsWin = `--data-raw "{""param1"":""value1"",""param2"":""value2""}"`;
+ const paramsWin = `--data-raw ^"{\\"param1\\":\\"value1\\",\\"param2\\":\\"value2\\"}^"`;
const paramsPosix = `--data-raw '{"param1":"value1","param2":"value2"}'`;
let curlCommand = Curl.generateCommand(data, "WINNT");
diff --git a/devtools/client/shared/test/xpcshell/test_curl.js b/devtools/client/shared/test/xpcshell/test_curl.js
index a2a6c3412ee..a04d0462fc9 100644
--- a/devtools/client/shared/test/xpcshell/test_curl.js
+++ b/devtools/client/shared/test/xpcshell/test_curl.js
@@ -230,18 +230,15 @@ add_task(async function () {
);
// Check binary data
- const dataBinaryPos = cmd.indexOf("--data-binary");
- const dataBinaryParam = `--data-binary ${isWin() ? "" : "$"}${escapeNewline(
- quote(request.postDataText)
- )}`;
+ const dataBinaryParam = `--data-binary \\\n $'------------14808\\r\\n`;
Assert.notStrictEqual(
- dataBinaryPos,
+ cmd.indexOf("--data-binary"),
-1,
"--data-binary param present in curl output"
);
- equal(
- cmd.substr(dataBinaryPos, dataBinaryParam.length),
- dataBinaryParam,
+
+ Assert.ok(
+ cmd.includes(dataBinaryParam),
"proper multipart data present in curl output"
);
});
@@ -353,14 +350,6 @@ function quote(str) {
return QUOTE + escaped + QUOTE;
}
-function escapeNewline(txt) {
- if (isWin()) {
- // Add `"` to close quote, then escape newline outside of quote, then start new quote
- return txt.replace(/[\r\n]{1,2}/g, '"^$&$&"');
- }
- return txt.replace(/\r/g, "\\r").replace(/\n/g, "\\n");
-}
-
// Header param is formatted as -H "Header: value" or -H 'Header: value'
function headerParam(h) {
return "-H " + quote(h);
Loading diff…
References
On This Page