Low firefox Logic Error 🔧 Commit mapped

Overview

Low
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactlow
DescriptionMitigation bypass in the Add-ons Manager component
ComponentToolkit
Bug ClassLogic Error
Tracker2045676
Fix commit2951644624f0 (firefox) +121/-9
CISA KEVNot listed
CreditedTomoya Nakanishi
Disclosed2026-08-18

Background

`verifyBundleSignedState`
an async function in the Add-ons Manager that opens an add-on bundle and returns its computed signature state and signed types.
`Package.verifySignedState`
the method that inspects an add-on package (XPI) and derives its signedState, which can throw when the file is malformed or unreadable.
`shouldVerifySignedState`
a helper that decides whether a given add-on type and location require a valid signature at all.
`SIGNEDSTATE_BROKEN`
the AddonManager signature-state value that marks an add-on as failing verification, causing it to be appDisabled and deactivated.

Root Cause Analysis

The vulnerable path is verifyBundleSignedState, which called Package.get(aBundle) and await pkg.verifySignedState(...) where the verification could throw if the underlying add-on file was corrupted or no longer a valid XPI. In the pre-fix code the only wrapper was a try/finally that guaranteed pkg.close() but did not catch the exception, so a throw propagated out of the function without ever resolving a signedState. This violated the invariant that every add-on subjected to verifySignatures() must end up with a concrete signature verdict; an add-on that should have been marked SIGNEDSTATE_BROKEN and disabled could instead skip the disabling logic entirely.

The fix wraps the whole open-verify-close sequence in a try/catch, and on any failure it consults shouldVerifySignedState: it returns SIGNEDSTATE_NOT_REQUIRED when signing is not mandatory and otherwise returns SIGNEDSTATE_BROKEN. Because a broken verdict now flows back to the caller, the add-on is correctly appDisabled and deactivated instead of remaining active.

Key insight
The single mistake was letting an exception from signature verification escape verifyBundleSignedState so that a signature-required add-on never received a SIGNEDSTATE_BROKEN verdict; the fix converts any verification failure into an explicit broken (or not-required) result so the disabling path always runs.

Attack Path

  1. Install a signed add-on A signed add-on is installed and initially verifies as SIGNEDSTATE_SIGNED and active.
  2. Corrupt the on-disk file The stored add-on file is replaced with content that is no longer a valid XPI, so verification will throw.
  3. Trigger re-verification A verifySignatures() pass runs and calls verifyBundleSignedState, whose verifySignedState call throws on the malformed file.
  4. Escape the verdict Pre-fix, the uncaught exception aborts before assigning a broken state, so the add-on is not moved into the disabled set.
  5. Retain active state The corrupted add-on stays active and enabled instead of being flagged broken, bypassing the intended signature mitigation.

Impact Assessment

An attacker gains a mitigation bypass in the Add-ons Manager whereby an add-on whose file fails signature verification can avoid being marked SIGNEDSTATE_BROKEN and thus stays active rather than being disabled. This occurs in the browser/toolkit (parent-process) add-on machinery and requires the ability to corrupt an installed add-on’s file and trigger a signature re-check; consistent with the metadata this is a low-severity logic error rather than a memory-safety or code-execution issue.

Changed Functions

FunctionChangeNotes
onPropertyChanged
toolkit/mozapps/extensions/test/xpcshell/test_signed_verify.js
modified

Files Changed

  • toolkit/mozapps/extensions/internal/XPIInstall.sys.mjs
  • toolkit/mozapps/extensions/test/xpcshell/test_signed_verify.js

Audit Directions

  • Unhandled verification exceptions
    Audit security-decision paths that await a throwing check and only use try/finally for cleanup; ensure a thrown error resolves to a safe, explicit denial rather than propagating past the decision.
  • Fail-open defaults
    Look for functions whose implicit failure behavior leaves state unchanged (still active/allowed); confirm the failure branch drives the object into the restrictive state such as SIGNEDSTATE_BROKEN.
  • Corrupted-input resilience
    Test signature and integrity checks against malformed or truncated on-disk files (not just tampered signatures) to confirm the add-on is disabled rather than silently retained.
diff --git a/toolkit/mozapps/extensions/internal/XPIInstall.sys.mjs b/toolkit/mozapps/extensions/internal/XPIInstall.sys.mjs
index 37af7d163d3..c63f5ad1481 100644
--- a/toolkit/mozapps/extensions/internal/XPIInstall.sys.mjs
+++ b/toolkit/mozapps/extensions/internal/XPIInstall.sys.mjs
@@ -935,16 +935,24 @@ function shouldVerifySignedState(aAddonType, aLocation) {
  *        or undefined if the file wasn't signed.
  */
 export var verifyBundleSignedState = async function (aBundle, aAddon) {
-  let pkg = Package.get(aBundle);
   try {
-    let { signedState, signedTypes } = await pkg.verifySignedState(
-      aAddon.id,
-      aAddon.type,
-      aAddon.location
-    );
-    return { signedState, signedTypes };
-  } finally {
-    pkg.close();
+    let pkg = Package.get(aBundle);
+    try {
+      let { signedState, signedTypes } = await pkg.verifySignedState(
+        aAddon.id,
+        aAddon.type,
+        aAddon.location
+      );
+      return { signedState, signedTypes };
+    } finally {
+      pkg.close();
+    }
+  } catch (e) {
+    logger.warn(`verifyBundleSignedState failed for ${aAddon.id}`, e);
+    if (!shouldVerifySignedState(aAddon.type, aAddon.location)) {
+      return { signedState: AddonManager.SIGNEDSTATE_NOT_REQUIRED };
+    }
+    return { signedState: AddonManager.SIGNEDSTATE_BROKEN };
   }
 };
 
diff --git a/toolkit/mozapps/extensions/test/xpcshell/test_signed_verify.js b/toolkit/mozapps/extensions/test/xpcshell/test_signed_verify.js
index d7f966262b0..b769b9301df 100644
--- a/toolkit/mozapps/extensions/test/xpcshell/test_signed_verify.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_signed_verify.js
@@ -644,3 +644,107 @@ add_task(
     await addon2.uninstall();
   }
 );
+
+add_task(useAMOStageCert(), async function test_broken_file() {
+  await promiseInstallFile(do_get_file(`${DATA}/signed1.xpi`));
+
+  let addon = await promiseAddonByID(ID);
+  Assert.notEqual(addon, null);
+  Assert.equal(addon.appDisabled, false);
+  Assert.equal(addon.isActive, true);
+  Assert.equal(addon.signedState, AddonManager.SIGNEDSTATE_SIGNED);
+
+  let file = AddonTestUtils.getFileForAddon(profileDir, ID);
+  await IOUtils.writeUTF8(file.path, "not a XPI file anymore");
+
+  let changedProperties = [];
+  let listener = {
+    onPropertyChanged(addon, properties) {
+      changedProperties.push(...properties);
+    },
+  };
+
+  AddonManager.addAddonListener(listener);
+
+  const disablePromise = promiseAddonEvent("onDisabling");
+  let changes;
+  const { messages } = await AddonTestUtils.promiseConsoleOutput(async () => {
+    changes = await verifySignatures();
+  });
+  await disablePromise;
+
+  Assert.equal(changes.enabled.length, 0);
+  Assert.equal(changes.disabled.length, 1);
+  Assert.equal(changes.disabled[0], ID);
+
+  Assert.deepEqual(
+    changedProperties,
+    ["signedState", "signedTypes", "appDisabled"],
+    "Got onPropertyChanged events for signedState and appDisabled"
+  );
+
+  Assert.ok(addon.appDisabled);
+  Assert.ok(!addon.isActive);
+  Assert.equal(addon.signedState, AddonManager.SIGNEDSTATE_BROKEN);
+
+  await addon.uninstall();
+  AddonManager.removeAddonListener(listener);
+
+  AddonTestUtils.checkMessages(messages, {
+    expected: [
+      { message: /verifyBundleSignedState failed for test@somewhere.com/ },
+    ],
+  });
+});
+
+// Verify that verifySignatures() does not change signedState for addons that
+// do not require signatures, even if the underlying file got corrupted.
+add_task(
+  {
+    ...useAMOStageCert(),
+    // # Non-extension add-ons are not supported on Android.
+    skip_if: () => AppConstants.platform == "android",
+  },
+  async function test_broken_file_not_requiring_signatures() {
+    // Note: If dictionaries ever require signatures (bug 1753276), change this
+    // test to another test case where shouldVerifySignedState returns false.
+    let addon = await promiseInstallWebExtension({
+      useAddonManager: true,
+      manifest: {
+        browser_specific_settings: { gecko: { id: "broken@dict" } },
+        dictionaries: { "en-US": "en-US.dic" },
+      },
+      files: { "en-US.dic": "", "en-US.aff": "" },
+    });
+    Assert.equal(addon.signedState, AddonManager.SIGNEDSTATE_NOT_REQUIRED);
+
+    let file = AddonTestUtils.getFileForAddon(profileDir, addon.id);
+    await IOUtils.writeUTF8(file.path, "not a XPI file anymore");
+
+    let listener = {
+      onPropertyChanged(_addon) {
+        Assert.ok(false, `Got unexpected onPropertyChanged for ${_addon.id}`);
+      },
+    };
+
+    AddonManager.addAddonListener(listener);
+
+    let changes;
+    const { messages } = await AddonTestUtils.promiseConsoleOutput(async () => {
+      changes = await verifySignatures();
+    });
+    Assert.equal(changes.enabled.length, 0);
+    Assert.equal(changes.disabled.length, 0);
+
+    Assert.equal(addon.appDisabled, false);
+    Assert.equal(addon.isActive, true);
+    Assert.equal(addon.signedState, AddonManager.SIGNEDSTATE_NOT_REQUIRED);
+
+    await addon.uninstall();
+    AddonManager.removeAddonListener(listener);
+
+    AddonTestUtils.checkMessages(messages, {
+      expected: [{ message: /verifyBundleSignedState failed for broken@dict/ }],
+    });
+  }
+);
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/toolkit/mozapps/extensions/test/xpcshell/test_signed_verify.js b/toolkit/mozapps/extensions/test/xpcshell/test_signed_verify.js
index d7f966262b0..b769b9301df 100644
--- a/toolkit/mozapps/extensions/test/xpcshell/test_signed_verify.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_signed_verify.js
@@ -644,3 +644,107 @@ add_task(
     await addon2.uninstall();
   }
 );
+
+add_task(useAMOStageCert(), async function test_broken_file() {
+  await promiseInstallFile(do_get_file(`${DATA}/signed1.xpi`));
+
+  let addon = await promiseAddonByID(ID);
+  Assert.notEqual(addon, null);
+  Assert.equal(addon.appDisabled, false);
+  Assert.equal(addon.isActive, true);
+  Assert.equal(addon.signedState, AddonManager.SIGNEDSTATE_SIGNED);
+
+  let file = AddonTestUtils.getFileForAddon(profileDir, ID);
+  await IOUtils.writeUTF8(file.path, "not a XPI file anymore");
+
+  let changedProperties = [];
+  let listener = {
+    onPropertyChanged(addon, properties) {
+      changedProperties.push(...properties);
+    },
+  };
+
+  AddonManager.addAddonListener(listener);
+
+  const disablePromise = promiseAddonEvent("onDisabling");
+  let changes;
+  const { messages } = await AddonTestUtils.promiseConsoleOutput(async () => {
+    changes = await verifySignatures();
+  });
+  await disablePromise;
+
+  Assert.equal(changes.enabled.length, 0);
+  Assert.equal(changes.disabled.length, 1);
+  Assert.equal(changes.disabled[0], ID);
+
+  Assert.deepEqual(
+    changedProperties,
+    ["signedState", "signedTypes", "appDisabled"],
+    "Got onPropertyChanged events for signedState and appDisabled"
+  );
+
+  Assert.ok(addon.appDisabled);
+  Assert.ok(!addon.isActive);
+  Assert.equal(addon.signedState, AddonManager.SIGNEDSTATE_BROKEN);
+
+  await addon.uninstall();
+  AddonManager.removeAddonListener(listener);
+
+  AddonTestUtils.checkMessages(messages, {
+    expected: [
+      { message: /verifyBundleSignedState failed for test@somewhere.com/ },
+    ],
+  });
+});
+
+// Verify that verifySignatures() does not change signedState for addons that
+// do not require signatures, even if the underlying file got corrupted.
+add_task(
+  {
+    ...useAMOStageCert(),
+    // # Non-extension add-ons are not supported on Android.
+    skip_if: () => AppConstants.platform == "android",
+  },
+  async function test_broken_file_not_requiring_signatures() {
+    // Note: If dictionaries ever require signatures (bug 1753276), change this
+    // test to another test case where shouldVerifySignedState returns false.
+    let addon = await promiseInstallWebExtension({
+      useAddonManager: true,
+      manifest: {
+        browser_specific_settings: { gecko: { id: "broken@dict" } },
+        dictionaries: { "en-US": "en-US.dic" },
+      },
+      files: { "en-US.dic": "", "en-US.aff": "" },
+    });
+    Assert.equal(addon.signedState, AddonManager.SIGNEDSTATE_NOT_REQUIRED);
+
+    let file = AddonTestUtils.getFileForAddon(profileDir, addon.id);
+    await IOUtils.writeUTF8(file.path, "not a XPI file anymore");
+
+    let listener = {
+      onPropertyChanged(_addon) {
+        Assert.ok(false, `Got unexpected onPropertyChanged for ${_addon.id}`);
+      },
+    };
+
+    AddonManager.addAddonListener(listener);
+
+    let changes;
+    const { messages } = await AddonTestUtils.promiseConsoleOutput(async () => {
+      changes = await verifySignatures();
+    });
+    Assert.equal(changes.enabled.length, 0);
+    Assert.equal(changes.disabled.length, 0);
+
+    Assert.equal(addon.appDisabled, false);
+    Assert.equal(addon.isActive, true);
+    Assert.equal(addon.signedState, AddonManager.SIGNEDSTATE_NOT_REQUIRED);
+
+    await addon.uninstall();
+    AddonManager.removeAddonListener(listener);
+
+    AddonTestUtils.checkMessages(messages, {
+      expected: [{ message: /verifyBundleSignedState failed for broken@dict/ }],
+    });
+  }
+);
Loading diff…