Low firefox Logic Error 🔧 Commit mapped

Overview

Low
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactlow
DescriptionPrivilege escalation in the Security component
ComponentToolkit
Bug ClassLogic Error
Tracker2032174
Fix commitb47f991a6430 (firefox) +151/-49
CISA KEVNot listed
Creditedpakhunov.anton.n
Disclosed2026-05-19

Changed Functions

FunctionChangeNotes
if
toolkit/modules/AsyncPrefs.sys.mjs
modified
set
toolkit/modules/AsyncPrefs.sys.mjs
modified
reset
toolkit/modules/AsyncPrefs.sys.mjs
modified
receiveMessage
toolkit/modules/AsyncPrefs.sys.mjs
modified
for
toolkit/modules/tests/browser/browser_AsyncPrefs.js
modified

Files Changed

  • toolkit/modules/AsyncPrefs.sys.mjs
  • toolkit/modules/tests/browser/browser_AsyncPrefs.js
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…