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

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
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…