Low firefox Logic Error 🔧 Commit mapped

Overview

Low
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactlow
DescriptionSpoofing issue in the Popup Blocker component
ComponentToolkit
Bug ClassLogic Error
Tracker2025170
Fix commitf30c6b61f356 (firefox) +135/-16
CISA KEVNot listed
CreditedSatoki Tsuji
Disclosed2026-05-19

Changed Functions

FunctionChangeNotes
add_task
browser/base/content/test/popups/browser_popup_blocker.js
modified
for
browser/base/content/test/popups/browser_popup_blocker.js
modified
showBlockedPopup
browser/modules/PopupAndRedirectBlockerObserver.sys.mjs
modified
for
toolkit/actors/PopupAndRedirectBlockingChild.sys.mjs
modified

Files Changed

  • browser/base/content/test/popups/browser_popup_blocker.js
  • browser/modules/PopupAndRedirectBlockerObserver.sys.mjs
  • toolkit/actors/PopupAndRedirectBlockingChild.sys.mjs
  • toolkit/actors/PopupAndRedirectBlockingParent.sys.mjs
diff --git a/browser/base/content/test/popups/browser_popup_blocker.js b/browser/base/content/test/popups/browser_popup_blocker.js
index ad741f87fce..45ec3a15064 100644
--- a/browser/base/content/test/popups/browser_popup_blocker.js
+++ b/browser/base/content/test/popups/browser_popup_blocker.js
@@ -157,7 +157,118 @@ add_task(async function test_dismissed_notification_switch_tabs() {
   gBrowser.removeTab(differentTab);
 });
 
-async function testPopupBlockingToolbar(tab) {
+// Bug 2025170.
+// "Allow" should unblock all popups across multiple browsing contexts
+// without erroring on out-of-bounds per-document indices.
+add_task(async function test_bug2025170_allow_all() {
+  const tab = await BrowserTestUtils.openNewForegroundTab(
+    gBrowser,
+    "https://example.com"
+  );
+
+  await SpecialPowers.spawn(
+    tab.linkedBrowser,
+    [baseURL + "popup_blocker.html"],
+    uri => {
+      for (let i = 0; i < 2; i++) {
+        let iframe = content.document.createElement("iframe");
+        iframe.src = uri;
+        content.document.body.appendChild(iframe);
+      }
+    }
+  );
+
+  // Because popup_blocker.html is calling window.open() with target
+  // window names, we expect only two new tabs.
+  await testPopupBlockingToolbar(
+    tab,
+    /*expectedBlocked=*/ 4,
+    /*expectedOpened=*/ 2
+  );
+});
+
+// Bug 2025170.
+// Make sure the correct popup is opened when the blocked popup list
+// spans multiple browsing contexts.
+add_task(async function test_bug2025170_unblock_popup() {
+  const tab = await BrowserTestUtils.openNewForegroundTab(
+    gBrowser,
+    "https://example.com"
+  );
+
+  await SpecialPowers.spawn(
+    tab.linkedBrowser,
+    [baseURL + "popup_blocker.html"],
+    uri => {
+      for (let i = 0; i < 2; i++) {
+        let iframe = content.document.createElement("iframe");
+        iframe.src = uri;
+        content.document.body.appendChild(iframe);
+      }
+    }
+  );
+
+  // Wait for the popup-blocked notification.
+  let notification;
+  await TestUtils.waitForCondition(
+    () =>
+      (notification = gBrowser
+        .getNotificationBox()
+        .getNotificationWithValue("popup-blocked"))
+  );
+
+  // Show the menu.
+  const popupShown = BrowserTestUtils.waitForEvent(window, "popupshown");
+  const popupFilled = waitForBlockedPopups(4, { doc: document });
+  EventUtils.synthesizeMouseAtCenter(
+    notification.buttonContainer.querySelector("button"),
+    {},
+    window
+  );
+
+  // Wait for the menu.
+  const popupEvent = await popupShown;
+  const menu = popupEvent.target;
+  is(menu.id, "blockedPopupOptions", "Blocked popup menu shown");
+
+  await popupFilled;
+
+  const popupItems = menu.querySelectorAll("[popupReportIndex]");
+  is(popupItems.length, 4, "Should have 4 blocked popup items");
+
+  // Track new tabs.
+  const popupTabs = [];
+  const onTabOpen = e => popupTabs.push(e.target);
+  gBrowser.tabContainer.addEventListener("TabOpen", onTabOpen);
+
+  // The last item in the flattened list appears first in DOM order.
+  // Without the fix, its popupReportIndex would be out of bounds.
+  popupItems[0].doCommand();
+
+  await TestUtils.waitForCondition(
+    () =>
+      popupTabs.length == 1 &&
+      popupTabs[0].linkedBrowser.currentURI.spec != "about:blank",
+    "Waiting for popup tab to open"
+  );
+  ok(
+    popupTabs[0].linkedBrowser.currentURI.spec.endsWith("popup_blocker_b.html"),
+    "Should have opened popup_blocker_b.html"
+  );
+
+  gBrowser.tabContainer.removeEventListener("TabOpen", onTabOpen);
+
+  // Clean up.
+  menu.hidePopup();
+  BrowserTestUtils.removeTab(popupTabs[0]);
+  BrowserTestUtils.removeTab(tab);
+});
+
+async function testPopupBlockingToolbar(
+  tab,
+  expectedBlocked = 2,
+  expectedOpened = 2
+) {
   let win = tab.ownerGlobal;
   // Wait for the popup-blocked notification.
   let notification;
@@ -170,7 +281,7 @@ async function testPopupBlockingToolbar(tab) {
 
   // Show the menu.
   let popupShown = BrowserTestUtils.waitForEvent(win, "popupshown");
-  let popupFilled = waitForBlockedPopups(2, {
+  let popupFilled = waitForBlockedPopups(expectedBlocked, {
     doc: win.document,
   });
   EventUtils.synthesizeMouseAtCenter(
@@ -196,7 +307,7 @@ async function testPopupBlockingToolbar(tab) {
   allow.doCommand();
   await TestUtils.waitForCondition(
     () =>
-      popupTabs.length == 2 &&
+      popupTabs.length == expectedOpened &&
       popupTabs.every(
         aTab => aTab.linkedBrowser.currentURI.spec != "about:blank"
       )
diff --git a/browser/modules/PopupAndRedirectBlockerObserver.sys.mjs b/browser/modules/PopupAndRedirectBlockerObserver.sys.mjs
index fcff357ce78..111fdf0a2a9 100644
--- a/browser/modules/PopupAndRedirectBlockerObserver.sys.mjs
+++ b/browser/modules/PopupAndRedirectBlockerObserver.sys.mjs
@@ -259,9 +259,11 @@ export var PopupAndRedirectBlockerObserver = {
       document.l10n.setAttributes(menuitem, "popup-show-popup-menuitem", {
         popupURI: blockedPopup.popupWindowURISpec,
       });
-      menuitem.setAttribute("popupReportIndex", i);
+      // The report index is the index into the blocked popup list
+      // maintained by the window where this popup was blocked.
+      menuitem.setAttribute("popupReportIndex", blockedPopup.reportIndex);
       // Store the source inner window id, so we can check if the document
-      // that triggered the redirect is still the same.
+      // that triggered the popup is still the same.
       menuitem.setAttribute("popupInnerWindowId", blockedPopup.innerWindowId);
       // Store the browser for the current tab. The active tab may change,
       // so we keep a reference to it.
@@ -341,12 +343,12 @@ export var PopupAndRedirectBlockerObserver = {
   showBlockedPopup(aEvent) {
     const { browser, browsingContext } = aEvent.target;
     const innerWindowId = aEvent.target.getAttribute("popupInnerWindowId");
-    const popupReportIndex = aEvent.target.getAttribute("popupReportIndex");
+    const reportIndex = aEvent.target.getAttribute("popupReportIndex");
 
     browser.popupAndRedirectBlocker.unblockPopup(
       browsingContext,
       innerWindowId,
-      popupReportIndex
+      reportIndex
     );
   },
 
diff --git a/toolkit/actors/PopupAndRedirectBlockingChild.sys.mjs b/toolkit/actors/PopupAndRedirectBlockingChild.sys.mjs
index 63af76b0fb1..a455edcc28a 100644
--- a/toolkit/actors/PopupAndRedirectBlockingChild.sys.mjs
+++ b/toolkit/actors/PopupAndRedirectBlockingChild.sys.mjs
@@ -91,11 +91,12 @@ export class PopupAndRedirectBlockingChild extends JSWindowActorChild {
     );
     const result = [];
 
-    for (let i = 0; i < length; ++i) {
-      const popup = state.popups[i];
+    for (let reportIndex = 0; reportIndex < length; ++reportIndex) {
+      const popup = state.popups[reportIndex];
       const { popupWindowURISpec } = popup;
       result.push({
         popupWindowURISpec,
+        reportIndex,
       });
     }
 
@@ -114,8 +115,8 @@ export class PopupAndRedirectBlockingChild extends JSWindowActorChild {
   }
 
   #unblockPopup(aMessage) {
-    const idx = aMessage.data.index;
-    const popup = this.#getOrCreateDocState().popups[idx];
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/browser/base/content/test/popups/browser_popup_blocker.js b/browser/base/content/test/popups/browser_popup_blocker.js
index ad741f87fce..45ec3a15064 100644
--- a/browser/base/content/test/popups/browser_popup_blocker.js
+++ b/browser/base/content/test/popups/browser_popup_blocker.js
@@ -157,7 +157,118 @@ add_task(async function test_dismissed_notification_switch_tabs() {
   gBrowser.removeTab(differentTab);
 });
 
-async function testPopupBlockingToolbar(tab) {
+// Bug 2025170.
+// "Allow" should unblock all popups across multiple browsing contexts
+// without erroring on out-of-bounds per-document indices.
+add_task(async function test_bug2025170_allow_all() {
+  const tab = await BrowserTestUtils.openNewForegroundTab(
+    gBrowser,
+    "https://example.com"
+  );
+
+  await SpecialPowers.spawn(
+    tab.linkedBrowser,
+    [baseURL + "popup_blocker.html"],
+    uri => {
+      for (let i = 0; i < 2; i++) {
+        let iframe = content.document.createElement("iframe");
+        iframe.src = uri;
+        content.document.body.appendChild(iframe);
+      }
+    }
+  );
+
+  // Because popup_blocker.html is calling window.open() with target
+  // window names, we expect only two new tabs.
+  await testPopupBlockingToolbar(
+    tab,
+    /*expectedBlocked=*/ 4,
+    /*expectedOpened=*/ 2
+  );
+});
+
+// Bug 2025170.
+// Make sure the correct popup is opened when the blocked popup list
+// spans multiple browsing contexts.
+add_task(async function test_bug2025170_unblock_popup() {
+  const tab = await BrowserTestUtils.openNewForegroundTab(
+    gBrowser,
+    "https://example.com"
+  );
+
+  await SpecialPowers.spawn(
+    tab.linkedBrowser,
+    [baseURL + "popup_blocker.html"],
+    uri => {
+      for (let i = 0; i < 2; i++) {
+        let iframe = content.document.createElement("iframe");
+        iframe.src = uri;
+        content.document.body.appendChild(iframe);
+      }
+    }
+  );
+
+  // Wait for the popup-blocked notification.
+  let notification;
+  await TestUtils.waitForCondition(
+    () =>
+      (notification = gBrowser
+        .getNotificationBox()
+        .getNotificationWithValue("popup-blocked"))
+  );
+
+  // Show the menu.
+  const popupShown = BrowserTestUtils.waitForEvent(window, "popupshown");
+  const popupFilled = waitForBlockedPopups(4, { doc: document });
+  EventUtils.synthesizeMouseAtCenter(
+    notification.buttonContainer.querySelector("button"),
+    {},
+    window
+  );
+
+  // Wait for the menu.
+  const popupEvent = await popupShown;
+  const menu = popupEvent.target;
+  is(menu.id, "blockedPopupOptions", "Blocked popup menu shown");
+
+  await popupFilled;
+
+  const popupItems = menu.querySelectorAll("[popupReportIndex]");
+  is(popupItems.length, 4, "Should have 4 blocked popup items");
+
+  // Track new tabs.
+  const popupTabs = [];
+  const onTabOpen = e => popupTabs.push(e.target);
+  gBrowser.tabContainer.addEventListener("TabOpen", onTabOpen);
+
+  // The last item in the flattened list appears first in DOM order.
+  // Without the fix, its popupReportIndex would be out of bounds.
+  popupItems[0].doCommand();
+
+  await TestUtils.waitForCondition(
+    () =>
+      popupTabs.length == 1 &&
+      popupTabs[0].linkedBrowser.currentURI.spec != "about:blank",
+    "Waiting for popup tab to open"
+  );
+  ok(
+    popupTabs[0].linkedBrowser.currentURI.spec.endsWith("popup_blocker_b.html"),
+    "Should have opened popup_blocker_b.html"
+  );
+
+  gBrowser.tabContainer.removeEventListener("TabOpen", onTabOpen);
+
+  // Clean up.
+  menu.hidePopup();
+  BrowserTestUtils.removeTab(popupTabs[0]);
+  BrowserTestUtils.removeTab(tab);
+});
+
+async function testPopupBlockingToolbar(
+  tab,
+  expectedBlocked = 2,
+  expectedOpened = 2
+) {
   let win = tab.ownerGlobal;
   // Wait for the popup-blocked notification.
   let notification;
@@ -170,7 +281,7 @@ async function testPopupBlockingToolbar(tab) {
 
   // Show the menu.
   let popupShown = BrowserTestUtils.waitForEvent(win, "popupshown");
-  let popupFilled = waitForBlockedPopups(2, {
+  let popupFilled = waitForBlockedPopups(expectedBlocked, {
     doc: win.document,
   });
   EventUtils.synthesizeMouseAtCenter(
@@ -196,7 +307,7 @@ async function testPopupBlockingToolbar(tab) {
   allow.doCommand();
   await TestUtils.waitForCondition(
     () =>
-      popupTabs.length == 2 &&
+      popupTabs.length == expectedOpened &&
       popupTabs.every(
         aTab => aTab.linkedBrowser.currentURI.spec != "about:blank"
       )
Loading diff…