Medium firefox Logic Error 🔧 Commit mapped

Overview

Medium
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactmoderate
DescriptionA missing delay in directory upload UI could have made it possible for an attacker to trick a user into granting permission via clickjacking.
ComponentDOM
Bug ClassLogic Error
Tracker1916659
Fix commitc67647610b53 (firefox) +61/-2
CISA KEVNot listed
CreditedShaheen Fazim
Disclosed2024-10-01

Changed Functions

FunctionChangeNotes
if
dom/filesystem/tests/script_promptHandler.js
modified
for
dom/filesystem/tests/script_promptHandler.js
modified

Files Changed

  • browser/base/content/test/tabPrompts/browser_confirmFolderUpload.js
  • browser/components/prompts/PromptCollection.sys.mjs
  • dom/filesystem/tests/script_promptHandler.js
diff --git a/browser/base/content/test/tabPrompts/browser_confirmFolderUpload.js b/browser/base/content/test/tabPrompts/browser_confirmFolderUpload.js
index a7e5d76fc40..d62edfd7ebb 100644
--- a/browser/base/content/test/tabPrompts/browser_confirmFolderUpload.js
+++ b/browser/base/content/test/tabPrompts/browser_confirmFolderUpload.js
@@ -107,8 +107,29 @@ async function testUploadPrompt(confirmUpload) {
     // Wait for confirmation prompt
     let prompt = await promptPromise;
     ok(prompt, "Shown upload confirmation prompt");
+
     is(prompt.ui.button0.label, "Upload", "Accept button label");
+    ok(
+      prompt.ui.button0.disabled,
+      "Accept button should be disabled by the security delay initially."
+    );
+
     ok(prompt.ui.button1.hasAttribute("default"), "Cancel is default button");
+    ok(
+      !prompt.ui.button1.disabled,
+      "Cancel button should not be disabled by the security delay."
+    );
+
+    info("Wait for the security delay to pass.");
+    let delayTime = Services.prefs.getIntPref("security.dialog_enable_delay");
+    // eslint-disable-next-line mozilla/no-arbitrary-setTimeout
+    await new Promise(resolve => setTimeout(resolve, delayTime + 100));
+
+    ok(
+      !prompt.ui.button0.disabled,
+      "Accept button should no longer be disabled."
+    );
+    ok(!prompt.ui.button1.disabled, "Cancel button should remain enabled.");
 
     // Close confirmation prompt
     await PromptTestUtils.handlePrompt(prompt, {
diff --git a/browser/components/prompts/PromptCollection.sys.mjs b/browser/components/prompts/PromptCollection.sys.mjs
index 85a7a10057a..ebb630fe152 100644
--- a/browser/components/prompts/PromptCollection.sys.mjs
+++ b/browser/components/prompts/PromptCollection.sys.mjs
@@ -195,7 +195,7 @@ export class PromptCollection {
         Services.prompt.MODAL_TYPE_TAB,
         title,
         message,
-        buttonFlags,
+        buttonFlags | Ci.nsIPrompt.BUTTON_DELAY_ENABLE,
         acceptLabel,
         null,
         null,
diff --git a/dom/filesystem/tests/script_promptHandler.js b/dom/filesystem/tests/script_promptHandler.js
index 235842b455d..9b412a85bb3 100644
--- a/dom/filesystem/tests/script_promptHandler.js
+++ b/dom/filesystem/tests/script_promptHandler.js
@@ -2,7 +2,45 @@
 
 let dialogObserverTopic = "common-dialog-loaded";
 
-function dialogObserver(subj) {
+function waitForButtonEnabledState(button) {
+  return new Promise(resolve => {
+    // Check if the button is already enabled (not disabled)
+    if (!button.disabled) {
+      resolve();
+      return;
+    }
+
+    // Create a MutationObserver instance
+    let win = button.ownerGlobal;
+    let { MutationObserver } = win;
+    const observer = new MutationObserver(mutationsList => {
+      for (const mutation of mutationsList) {
+        if (
+          mutation.type === "attributes" &&
+          mutation.attributeName === "disabled"
+        ) {
+          if (!button.disabled) {
+            // Resolve the promise when the button is enabled
+            observer.disconnect(); // Stop observing
+            resolve();
+          }
+        }
+      }
+    });
+
+    // Start observing the button for changes to the 'disabled' attribute
+    observer.observe(button, {
+      attributes: true,
+      attributeFilter: ["disabled"],
+    });
+  });
+}
+
+async function dialogObserver(subj) {
+  let dialog = subj.document.querySelector("dialog");
+  let acceptButton = dialog.getButton("accept");
+  await waitForButtonEnabledState(acceptButton);
+
   subj.document.querySelector("dialog").acceptDialog();
   sendAsyncMessage("promptAccepted");
 }
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/browser/base/content/test/tabPrompts/browser_confirmFolderUpload.js b/browser/base/content/test/tabPrompts/browser_confirmFolderUpload.js
index a7e5d76fc40..d62edfd7ebb 100644
--- a/browser/base/content/test/tabPrompts/browser_confirmFolderUpload.js
+++ b/browser/base/content/test/tabPrompts/browser_confirmFolderUpload.js
@@ -107,8 +107,29 @@ async function testUploadPrompt(confirmUpload) {
     // Wait for confirmation prompt
     let prompt = await promptPromise;
     ok(prompt, "Shown upload confirmation prompt");
+
     is(prompt.ui.button0.label, "Upload", "Accept button label");
+    ok(
+      prompt.ui.button0.disabled,
+      "Accept button should be disabled by the security delay initially."
+    );
+
     ok(prompt.ui.button1.hasAttribute("default"), "Cancel is default button");
+    ok(
+      !prompt.ui.button1.disabled,
+      "Cancel button should not be disabled by the security delay."
+    );
+
+    info("Wait for the security delay to pass.");
+    let delayTime = Services.prefs.getIntPref("security.dialog_enable_delay");
+    // eslint-disable-next-line mozilla/no-arbitrary-setTimeout
+    await new Promise(resolve => setTimeout(resolve, delayTime + 100));
+
+    ok(
+      !prompt.ui.button0.disabled,
+      "Accept button should no longer be disabled."
+    );
+    ok(!prompt.ui.button1.disabled, "Cancel button should remain enabled.");
 
     // Close confirmation prompt
     await PromptTestUtils.handlePrompt(prompt, {
diff --git a/dom/filesystem/tests/script_promptHandler.js b/dom/filesystem/tests/script_promptHandler.js
index 235842b455d..9b412a85bb3 100644
--- a/dom/filesystem/tests/script_promptHandler.js
+++ b/dom/filesystem/tests/script_promptHandler.js
@@ -2,7 +2,45 @@
 
 let dialogObserverTopic = "common-dialog-loaded";
 
-function dialogObserver(subj) {
+function waitForButtonEnabledState(button) {
+  return new Promise(resolve => {
+    // Check if the button is already enabled (not disabled)
+    if (!button.disabled) {
+      resolve();
+      return;
+    }
+
+    // Create a MutationObserver instance
+    let win = button.ownerGlobal;
+    let { MutationObserver } = win;
+    const observer = new MutationObserver(mutationsList => {
+      for (const mutation of mutationsList) {
+        if (
+          mutation.type === "attributes" &&
+          mutation.attributeName === "disabled"
+        ) {
+          if (!button.disabled) {
+            // Resolve the promise when the button is enabled
+            observer.disconnect(); // Stop observing
+            resolve();
+          }
+        }
+      }
+    });
+
+    // Start observing the button for changes to the 'disabled' attribute
+    observer.observe(button, {
+      attributes: true,
+      attributeFilter: ["disabled"],
+    });
+  });
+}
+
+async function dialogObserver(subj) {
+  let dialog = subj.document.querySelector("dialog");
+  let acceptButton = dialog.getButton("accept");
+  await waitForButtonEnabledState(acceptButton);
+
   subj.document.querySelector("dialog").acceptDialog();
   sendAsyncMessage("promptAccepted");
 }
Loading diff…