Firefox · Toolkit
CVE-2026-8970
Logic Error in Toolkit
Overview
Low
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
iftoolkit/modules/AsyncPrefs.sys.mjs |
modified | |
settoolkit/modules/AsyncPrefs.sys.mjs |
modified | |
resettoolkit/modules/AsyncPrefs.sys.mjs |
modified | |
receiveMessagetoolkit/modules/AsyncPrefs.sys.mjs |
modified | |
fortoolkit/modules/tests/browser/browser_AsyncPrefs.js |
modified |
Files Changed
toolkit/modules/AsyncPrefs.sys.mjstoolkit/modules/tests/browser/browser_AsyncPrefs.js
Patch
diff --git a/toolkit/modules/AsyncPrefs.sys.mjs b/toolkit/modules/AsyncPrefs.sys.mjs
index 876d7fe7d14..8932738b245 100644
--- a/toolkit/modules/AsyncPrefs.sys.mjs
+++ b/toolkit/modules/AsyncPrefs.sys.mjs
@@ -5,7 +5,7 @@
const kInChildProcess =
Services.appinfo.processType == Services.appinfo.PROCESS_TYPE_CONTENT;
-const kAllowedPrefs = new Set([
+const kPrivilegedAboutPrefs = new Set([
// NB: please leave the testing prefs at the top, and sort the rest alphabetically if you add
// anything.
"testing.allowed-prefs.some-bool-pref",
@@ -14,7 +14,16 @@ const kAllowedPrefs = new Set([
"browser.contentblocking.report.hide_vpn_banner",
"browser.contentblocking.report.show_mobile_app",
+]);
+/**
+ * This set of prefs is exposed to web content processes. By default,
+ * AsyncPrefs is obviously only available to privileged code, but in the
+ * case of a compromised content process, we would still want to avoid it
+ * being able to set security-relevant prefs. If in doubt, talk to the
+ * security team before adding more prefs to this list.
+ */
+const kUnprivilegedExposedPrefs = new Set([
"narrate.rate",
"narrate.voice",
@@ -40,16 +49,37 @@ const kPrefTypeMap = new Map([
["string", Services.prefs.PREF_STRING],
]);
-function maybeReturnErrorForReset(pref) {
- if (!kAllowedPrefs.has(pref)) {
- return `Resetting pref ${pref} from content is not allowed.`;
+function maybeReturnErrorForOperation(operation, pref, remoteType) {
+ let isPrivilegedRemote =
+ remoteType == "privilegedabout" || remoteType == "parent";
+ let isUnprivilegedRemote =
+ remoteType == "file" ||
+ remoteType == "web" ||
+ remoteType.startsWith("webIsolated=");
+ if (!isPrivilegedRemote && !isUnprivilegedRemote) {
+ return `Unknown remote type ${remoteType} when trying to ${operation} pref ${pref}.`;
+ }
+ if (
+ isPrivilegedRemote &&
+ !kPrivilegedAboutPrefs.has(pref) &&
+ !kUnprivilegedExposedPrefs.has(pref)
+ ) {
+ return `Not allowed to ${operation} pref ${pref} from ${remoteType} process.`;
+ }
+ if (isUnprivilegedRemote && !kUnprivilegedExposedPrefs.has(pref)) {
+ return `Not allowed to ${operation} pref ${pref} from ${remoteType} process.`;
}
return false;
}
-function maybeReturnErrorForSet(pref, value) {
- if (!kAllowedPrefs.has(pref)) {
- return `Setting pref ${pref} from content is not allowed.`;
+function maybeReturnErrorForReset(pref, remoteType = "web") {
+ return maybeReturnErrorForOperation("reset", pref, remoteType);
+}
+
+function maybeReturnErrorForSet(pref, value, remoteType = "web") {
+ let error = maybeReturnErrorForOperation("set", pref, remoteType);
+ if (error) {
+ return error;
}
let valueType = typeof value;
@@ -68,7 +98,11 @@ function maybeReturnErrorForSet(pref, value) {
export class AsyncPrefsChild extends JSProcessActorChild {
set(pref, value) {
- let error = maybeReturnErrorForSet(pref, value);
+ let error = maybeReturnErrorForSet(
+ pref,
+ value,
+ Services.appinfo.remoteType
+ );
if (error) {
return Promise.reject(error);
}
@@ -80,7 +114,7 @@ export class AsyncPrefsChild extends JSProcessActorChild {
}
reset(pref) {
- let error = maybeReturnErrorForReset(pref);
+ let error = maybeReturnErrorForReset(pref, Services.appinfo.remoteType);
if (error) {
return Promise.reject(error);
}
@@ -96,14 +130,14 @@ export var AsyncPrefs = {
.getActor("AsyncPrefs")
.set(pref, value);
}
- return AsyncPrefsParent.set(pref, value);
+ return AsyncPrefsParent.set(pref, value, "parent");
},
reset(pref) {
if (kInChildProcess) {
return ChromeUtils.domProcessChild.getActor("AsyncPrefs").reset(pref);
}
- return AsyncPrefsParent.reset(pref);
+ return AsyncPrefsParent.reset(pref, "parent");
},
};
@@ -114,8 +148,8 @@ const methodForType = {
};
export class AsyncPrefsParent extends JSProcessActorParent {
- static set(pref, value) {
- let error = maybeReturnErrorForSet(pref, value);
+ static set(pref, value, remoteType) {
+ let error = maybeReturnErrorForSet(pref, value, remoteType);
if (error) {
return Promise.reject(error);
}
@@ -130,8 +164,8 @@ export class AsyncPrefsParent extends JSProcessActorParent {
return Promise.resolve(value);
}
- static reset(pref) {
- let error = maybeReturnErrorForReset(pref);
+ static reset(pref, remoteType) {
+ let error = maybeReturnErrorForReset(pref, remoteType);
if (error) {
return Promise.reject(error);
}
@@ -148,8 +182,12 @@ export class AsyncPrefsParent extends JSProcessActorParent {
receiveMessage(msg) {
if (msg.name == "AsyncPrefs:SetPref") {
- return AsyncPrefsParent.set(msg.data.pref, msg.data.value);
+ return AsyncPrefsParent.set(
+ msg.data.pref,
+ msg.data.value,
+ this.manager.remoteType
+ );
}
- return AsyncPrefsParent.reset(msg.data.pref);
+ return AsyncPrefsParent.reset(msg.data.pref, this.manager.remoteType);
}
}
diff --git a/toolkit/modules/tests/browser/browser_AsyncPrefs.js b/toolkit/modules/tests/browser/browser_AsyncPrefs.js
index 96eadc4b2e3..194f751c010 100644
--- a/toolkit/modules/tests/browser/browser_AsyncPrefs.js
+++ b/toolkit/modules/tests/browser/browser_AsyncPrefs.js
@@ -1,11 +1,20 @@
"use strict";
-const kWhiteListedBool = "testing.allowed-prefs.some-bool-pref";
-const kWhiteListedChar = "testing.allowed-prefs.some-char-pref";
-const kWhiteListedInt = "testing.allowed-prefs.some-int-pref";
+const kBoolTestPref = "testing.allowed-prefs.some-bool-pref";
+const kCharTestPref = "testing.allowed-prefs.some-char-pref";
+const kIntTestPref = "testing.allowed-prefs.some-int-pref";
+
+// We have to use a real pref because we don't want to include testing
+// prefs for the web content process.
+const kRealTestPref = "reader.font_size";
function resetPrefs() {
- for (let pref of [kWhiteListedBool, kWhiteListedChar, kWhiteListedBool]) {
+ for (let pref of [
+ kBoolTestPref,
+ kCharTestPref,
+ kIntTestPref,
+ kRealTestPref,
+ ]) {
Services.prefs.clearUserPref(pref);
}
}
@@ -26,16 +35,26 @@ async function runTest() {
let { AsyncPrefs } = ChromeUtils.importESModule(
"resource://gre/modules/AsyncPrefs.sys.mjs"
);
- const kInChildProcess =
- Services.appinfo.processType == Services.appinfo.PROCESS_TYPE_CONTENT;
// Need to define these again because when run in a content task we have no scope access.
- const kNotWhiteListed = "some.pref.thats.not.whitelisted";
- const kWhiteListedBool = "testing.allowed-prefs.some-bool-pref";
- const kWhiteListedChar = "testing.allowed-prefs.some-char-pref";
- const kWhiteListedInt = "testing.allowed-prefs.some-int-pref";
-
- const procDesc = kInChildProcess ? "child process" : "parent process";
+ const kNotAllowed = "some.pref.thats.not.allowed";
+ const kBoolTestPref = "testing.allowed-prefs.some-bool-pref";
+ const kCharTestPref = "testing.allowed-prefs.some-char-pref";
+ const kIntTestPref = "testing.allowed-prefs.some-int-pref";
+ const kRealTestPref = "reader.font_size";
+
+ let procDesc;
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/toolkit/modules/tests/browser/browser_AsyncPrefs.js b/toolkit/modules/tests/browser/browser_AsyncPrefs.js
index 96eadc4b2e3..194f751c010 100644
--- a/toolkit/modules/tests/browser/browser_AsyncPrefs.js
+++ b/toolkit/modules/tests/browser/browser_AsyncPrefs.js
@@ -1,11 +1,20 @@
"use strict";
-const kWhiteListedBool = "testing.allowed-prefs.some-bool-pref";
-const kWhiteListedChar = "testing.allowed-prefs.some-char-pref";
-const kWhiteListedInt = "testing.allowed-prefs.some-int-pref";
+const kBoolTestPref = "testing.allowed-prefs.some-bool-pref";
+const kCharTestPref = "testing.allowed-prefs.some-char-pref";
+const kIntTestPref = "testing.allowed-prefs.some-int-pref";
+
+// We have to use a real pref because we don't want to include testing
+// prefs for the web content process.
+const kRealTestPref = "reader.font_size";
function resetPrefs() {
- for (let pref of [kWhiteListedBool, kWhiteListedChar, kWhiteListedBool]) {
+ for (let pref of [
+ kBoolTestPref,
+ kCharTestPref,
+ kIntTestPref,
+ kRealTestPref,
+ ]) {
Services.prefs.clearUserPref(pref);
}
}
@@ -26,16 +35,26 @@ async function runTest() {
let { AsyncPrefs } = ChromeUtils.importESModule(
"resource://gre/modules/AsyncPrefs.sys.mjs"
);
- const kInChildProcess =
- Services.appinfo.processType == Services.appinfo.PROCESS_TYPE_CONTENT;
// Need to define these again because when run in a content task we have no scope access.
- const kNotWhiteListed = "some.pref.thats.not.whitelisted";
- const kWhiteListedBool = "testing.allowed-prefs.some-bool-pref";
- const kWhiteListedChar = "testing.allowed-prefs.some-char-pref";
- const kWhiteListedInt = "testing.allowed-prefs.some-int-pref";
-
- const procDesc = kInChildProcess ? "child process" : "parent process";
+ const kNotAllowed = "some.pref.thats.not.allowed";
+ const kBoolTestPref = "testing.allowed-prefs.some-bool-pref";
+ const kCharTestPref = "testing.allowed-prefs.some-char-pref";
+ const kIntTestPref = "testing.allowed-prefs.some-int-pref";
+ const kRealTestPref = "reader.font_size";
+
+ let procDesc;
+ switch (Services.appinfo.remoteType) {
+ case null:
+ procDesc = "parent process";
+ break;
+ case "privilegedabout":
+ procDesc = "privileged about: process";
+ break;
+ default:
+ procDesc = `${Services.appinfo.remoteType} child process`;
+ break;
+ }
const valueResultMap = [
[true, "Bool"],
@@ -50,9 +69,9 @@ async function runTest() {
];
const prefMap = [
- ["Bool", kWhiteListedBool],
- ["Char", kWhiteListedChar],
- ["Int", kWhiteListedInt],
+ ["Bool", kBoolTestPref],
+ ["Char", kCharTestPref],
+ ["Int", kIntTestPref],
];
function doesFail(pref, value) {
@@ -80,23 +99,26 @@ async function runTest() {
}
for (let [val] of valueResultMap) {
- await doesFail(kNotWhiteListed, val);
+ await doesFail(kNotAllowed, val);
is(
- Services.prefs.prefHasUserValue(kNotWhiteListed),
+ Services.prefs.prefHasUserValue(kNotAllowed),
false,
"Pref shouldn't get changed"
);
}
- let resetMsg = `Should not succeed resetting ${kNotWhiteListed} in ${procDesc}`;
- AsyncPrefs.reset(kNotWhiteListed).then(
+ let resetMsg = `Should not succeed resetting ${kNotAllowed} in ${procDesc}`;
+ AsyncPrefs.reset(kNotAllowed).then(
() => ok(false, resetMsg),
error => ok(true, resetMsg + "; " + error)
);
+ let haveSomePrivilege =
+ Services.appinfo.remoteType == null ||
+ Services.appinfo.remoteType == "privilegedabout";
for (let [type, pref] of prefMap) {
for (let [val, result] of valueResultMap) {
- if (result == type) {
+ if (haveSomePrivilege && result == type) {
await doesWork(pref, val);
is(
Services.prefs["get" + type + "Pref"](pref),
@@ -114,20 +136,62 @@ async function runTest() {
}
}
}
+
+ let oldValue = Services.prefs.getIntPref(kRealTestPref);
+ await AsyncPrefs.set(kRealTestPref, 2 * oldValue);
+ Assert.equal(
+ Services.prefs.getIntPref(kRealTestPref),
+ 2 * oldValue,
+ `Should have been able to set ${kRealTestPref} from ${procDesc}`
+ );
+ await AsyncPrefs.reset(kRealTestPref);
+ Assert.equal(
+ Services.prefs.getIntPref(kRealTestPref),
+ oldValue,
+ `Should have been able to reset ${kRealTestPref} from ${procDesc}`
+ );
}
-add_task(async function runInParent() {
- await runTest();
- resetPrefs();
-});
+describe("AsyncPrefs", function runInParent() {
+ afterEach(resetPrefs);
+
+ it("should work in the parent process", async function runInParent() {
+ await runTest();
+ });
+
+ it("should work in the privileged about process", async function runInPrivilegedAbout() {
+ await BrowserTestUtils.withNewTab(
+ "about:privatebrowsing",
+ async function (browser) {
+ ok(
+ browser.isRemoteBrowser,
+ "Should actually run this in child process"
+ );
+ Assert.equal(
+ browser.browsingContext.currentRemoteType,
+ "privilegedabout",
+ "Should be in a privileged about process"
+ );
+ await SpecialPowers.spawn(browser, [], runTest);
+ }
+ );
+ });
-if (gMultiProcessBrowser) {
- add_task(async function runInChild() {
- ok(
- gBrowser.selectedBrowser.isRemoteBrowser,
- "Should actually run this in child process"
+ it("should work in the web child process", async function runInWebChild() {
+ await BrowserTestUtils.withNewTab(
+ "https://example.com/somewhere404",
+ async function (browser) {
+ ok(
+ browser.isRemoteBrowser,
+ "Should actually run this in child process"
+ );
+ Assert.equal(
+ browser.browsingContext.currentRemoteType,
+ "webIsolated=https://example.com",
+ "Should be in a web isolated process"
+ );
+ await SpecialPowers.spawn(browser, [], runTest);
+ }
);
- await SpecialPowers.spawn(gBrowser.selectedBrowser, [], runTest);
- resetPrefs();
});
-}
+});
Loading diff…
References
On This Page