Medium firefox Logic Error 🔧 Commit mapped

Overview

Medium
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactmoderate
DescriptionWhen a file download is specified via the <code>Content-Disposition</code> header, that directive would be ignored if the file was included via a <code>&lt;embed&gt;</code> or <code>&lt;object&gt;</code> tag, potentially making a website vulnerable to a cross-site scripting attack.
ComponentDOM
Bug ClassLogic Error
Tracker1971140
Fix commit283226060d9e (firefox) +174/-142
CISA KEVNot listed
CreditedDaniil Satyaev (Positive Technologies)
Disclosed2025-06-24

Changed Functions

FunctionChangeNotes
if
docshell/base/nsDocShell.cpp
modified
add_task
dom/base/test/browser_object_attachment.js
modified
if
dom/base/test/browser_object_attachment.js
modified
onDownloadAdded
dom/base/test/browser_object_attachment.js
modified

Files Changed

  • docshell/base/nsDocShell.cpp
  • dom/base/test/browser.toml
  • dom/base/test/browser_object_attachment.js
  • dom/base/test/file_html_attachment.html
  • dom/base/test/file_html_attachment.html^headers^
  • dom/base/test/file_html_object_attachment.html
  • dom/base/test/file_svg_attachment.svg
  • dom/base/test/file_svg_attachment.svg^headers^
  • dom/base/test/file_svg_object_attachment.html
  • uriloader/base/nsIURILoader.idl
  • uriloader/base/nsURILoader.cpp
diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp
index 945e2080c5a..8875bfe59d8 100644
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -10938,11 +10938,14 @@ static nsresult AppendSegmentToString(nsIInputStream* aIn, void* aClosure,
     openFlags |= nsIURILoader::DONT_RETARGET;
   }
 
-  // Unless the pref is set, object/embed loads always specify DONT_RETARGET.
-  // See bug 1868001 for details.
-  if (!aIsDocumentLoad &&
-      !StaticPrefs::dom_navigation_object_embed_allow_retargeting()) {
-    openFlags |= nsIURILoader::DONT_RETARGET;
+  if (!aIsDocumentLoad) {
+    openFlags |= nsIURILoader::IS_OBJECT_EMBED;
+
+    // Unless the pref is set, object/embed loads always specify DONT_RETARGET.
+    // See bug 1868001 for details.
+    if (!StaticPrefs::dom_navigation_object_embed_allow_retargeting()) {
+      openFlags |= nsIURILoader::DONT_RETARGET;
+    }
   }
 
   return openFlags;
@@ -11033,8 +11036,14 @@ nsresult nsDocShell::OpenRedirectedChannel(nsDocShellLoadState* aLoadState) {
   // ClientInfo, so we just need to allocate a corresponding ClientSource.
   CreateReservedSourceIfNeeded(channel, GetMainThreadSerialEventTarget());
 
+  uint32_t documentOpenInfoFlags = nsIURILoader::DONT_RETARGET;
+  if (loadInfo->GetExternalContentPolicyType() ==
+      ExtContentPolicy::TYPE_OBJECT) {
+    documentOpenInfoFlags |= nsIURILoader::IS_OBJECT_EMBED;
+  }
+
   RefPtr<nsDocumentOpenInfo> loader =
-      new nsDocumentOpenInfo(this, nsIURILoader::DONT_RETARGET, nullptr);
+      new nsDocumentOpenInfo(this, documentOpenInfoFlags, nullptr);
   channel->SetLoadGroup(mLoadGroup);
 
   MOZ_ALWAYS_SUCCEEDS(loader->Prepare());
diff --git a/dom/base/test/browser.toml b/dom/base/test/browser.toml
index 89d4183bbd2..799fd8b1a81 100644
--- a/dom/base/test/browser.toml
+++ b/dom/base/test/browser.toml
@@ -123,6 +123,12 @@ support-files = [
   "file_pdf_object_attachment.html",
   "file_pdf_attachment.pdf",
   "file_pdf_attachment.pdf^headers^",
+  "file_svg_object_attachment.html",
+  "file_svg_attachment.svg",
+  "file_svg_attachment.svg^headers^",
+  "file_html_object_attachment.html",
+  "file_html_attachment.html",
+  "file_html_attachment.html^headers^",
 ]
 
 ["browser_outline_refocus.js"]
diff --git a/dom/base/test/browser_object_attachment.js b/dom/base/test/browser_object_attachment.js
index b4432862f0a..851fffae565 100644
--- a/dom/base/test/browser_object_attachment.js
+++ b/dom/base/test/browser_object_attachment.js
@@ -7,29 +7,75 @@ const httpsTestRoot = getRootDirectory(gTestPath).replace(
   "https://example.com"
 );
 
-add_task(async function test_pdf_object_attachment() {
-  await SpecialPowers.pushPrefEnv({
-    set: [["dom.navigation.object_embed.allow_retargeting", false]],
-  });
+async function loadAndCheck(file, displayInline, downloadFile = null) {
+  // Get the downloads list and add a view to listen for a download to be added.
+  // We do this even if we aren't going to download anything, so we notice if a
+  // download is started.
+  let download;
+  let downloadList = await Downloads.getList(Downloads.ALL);
+  let downloadView = {
+    async onDownloadAdded(aDownload) {
+      info("download added");
+      ok(downloadFile, "Should be expecting a download");
+      download = aDownload;
+
+      // Clean up the download from the list
+      downloadList.remove(aDownload);
+      await aDownload.finalize(true);
+    },
+  };
+  await downloadList.addView(downloadView);
 
+  // Open the new URL and perform the load.
   await BrowserTestUtils.withNewTab(
-    `${httpsTestRoot}/file_pdf_object_attachment.html`,
+    `${httpsTestRoot}/${file}`,
     async browser => {
       is(
         browser.browsingContext.children.length,
-        1,
-        "Should have a child frame"
+        displayInline ? 1 : 0,
+        `Should ${displayInline ? "not " : ""}have a child frame`
+      );
+
+      await SpecialPowers.spawn(
+        browser,
+        [displayInline],
+        async displayInline => {
+          let obj = content.document.querySelector("object");
+          is(
+            obj.displayedType,
+            displayInline
+              ? Ci.nsIObjectLoadingContent.TYPE_DOCUMENT
+              : Ci.nsIObjectLoadingContent.TYPE_FALLBACK,
+            `should be displaying TYPE_${displayInline ? "DOCUMENT" : "FALLBACK"}`
+          );
+        }
       );
-      await SpecialPowers.spawn(browser, [], async () => {
-        let obj = content.document.querySelector("object");
-        is(
-          obj.displayedType,
-          Ci.nsIObjectLoadingContent.TYPE_DOCUMENT,
-          "should be displaying TYPE_DOCUMENT"
-        );
-      });
     }
   );
+
+  // Clean up our download observer.
+  await downloadList.removeView(downloadView);
+  if (downloadFile) {
+    is(
+      download.source.url,
+      `${httpsTestRoot}/${downloadFile}`,
+      "Download has the correct source"
+    );
+  } else {
+    is(download, undefined, "Should not have seen a download");
+  }
+}
+
+add_task(async function test_pdf_object_attachment() {
+  await SpecialPowers.pushPrefEnv({
+    set: [
+      ["dom.navigation.object_embed.allow_retargeting", false],
+      ["browser.download.open_pdf_attachments_inline", false],
+    ],
+  });
+
+  // PDF attachment should display inline.
+  await loadAndCheck("file_pdf_object_attachment.html", true);
 });
 
 add_task(async function test_img_object_attachment() {
@@ -37,132 +83,71 @@ add_task(async function test_img_object_attachment() {
     set: [["dom.navigation.object_embed.allow_retargeting", false]],
   });
 
-  await BrowserTestUtils.withNewTab(
-    `${httpsTestRoot}/file_img_object_attachment.html`,
-    async browser => {
-      is(
-        browser.browsingContext.children.length,
-        1,
-        "Should have a child frame"
-      );
-      await SpecialPowers.spawn(browser, [], async () => {
-        let obj = content.document.querySelector("object");
-        is(
-          obj.displayedType,
-          Ci.nsIObjectLoadingContent.TYPE_DOCUMENT,
-          "should be displaying TYPE_DOCUMENT"
-        );
-      });
-    }
-  );
+  // Image attachment should display inline.
+  await loadAndCheck("file_img_object_attachment.html", true);
 });
 
-async function waitForDownload() {
-  // Get the downloads list and add a view to listen for a download to be added.
-  let downloadList = await Downloads.getList(Downloads.ALL);
+add_task(async function test_svg_object_attachment() {
+  await SpecialPowers.pushPrefEnv({
+    set: [["dom.navigation.object_embed.allow_retargeting", false]],
+  });
 
-  // Wait for a single download
-  let downloadView;
-  let finishedAllDownloads = new Promise(resolve => {
-    downloadView = {
-      onDownloadAdded(aDownload) {
-        info("download added");
-        resolve(aDownload);
-      },
-    };
+  // SVG attachment should fail to load.
+  await loadAndCheck("file_svg_object_attachment.html", false);
+});
+
+add_task(async function test_html_object_attachment() {
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/dom/base/test/browser.toml b/dom/base/test/browser.toml
index 89d4183bbd2..799fd8b1a81 100644
--- a/dom/base/test/browser.toml
+++ b/dom/base/test/browser.toml
@@ -123,6 +123,12 @@ support-files = [
   "file_pdf_object_attachment.html",
   "file_pdf_attachment.pdf",
   "file_pdf_attachment.pdf^headers^",
+  "file_svg_object_attachment.html",
+  "file_svg_attachment.svg",
+  "file_svg_attachment.svg^headers^",
+  "file_html_object_attachment.html",
+  "file_html_attachment.html",
+  "file_html_attachment.html^headers^",
 ]
 
 ["browser_outline_refocus.js"]
diff --git a/dom/base/test/browser_object_attachment.js b/dom/base/test/browser_object_attachment.js
index b4432862f0a..851fffae565 100644
--- a/dom/base/test/browser_object_attachment.js
+++ b/dom/base/test/browser_object_attachment.js
@@ -7,29 +7,75 @@ const httpsTestRoot = getRootDirectory(gTestPath).replace(
   "https://example.com"
 );
 
-add_task(async function test_pdf_object_attachment() {
-  await SpecialPowers.pushPrefEnv({
-    set: [["dom.navigation.object_embed.allow_retargeting", false]],
-  });
+async function loadAndCheck(file, displayInline, downloadFile = null) {
+  // Get the downloads list and add a view to listen for a download to be added.
+  // We do this even if we aren't going to download anything, so we notice if a
+  // download is started.
+  let download;
+  let downloadList = await Downloads.getList(Downloads.ALL);
+  let downloadView = {
+    async onDownloadAdded(aDownload) {
+      info("download added");
+      ok(downloadFile, "Should be expecting a download");
+      download = aDownload;
+
+      // Clean up the download from the list
+      downloadList.remove(aDownload);
+      await aDownload.finalize(true);
+    },
+  };
+  await downloadList.addView(downloadView);
 
+  // Open the new URL and perform the load.
   await BrowserTestUtils.withNewTab(
-    `${httpsTestRoot}/file_pdf_object_attachment.html`,
+    `${httpsTestRoot}/${file}`,
     async browser => {
       is(
         browser.browsingContext.children.length,
-        1,
-        "Should have a child frame"
+        displayInline ? 1 : 0,
+        `Should ${displayInline ? "not " : ""}have a child frame`
+      );
+
+      await SpecialPowers.spawn(
+        browser,
+        [displayInline],
+        async displayInline => {
+          let obj = content.document.querySelector("object");
+          is(
+            obj.displayedType,
+            displayInline
+              ? Ci.nsIObjectLoadingContent.TYPE_DOCUMENT
+              : Ci.nsIObjectLoadingContent.TYPE_FALLBACK,
+            `should be displaying TYPE_${displayInline ? "DOCUMENT" : "FALLBACK"}`
+          );
+        }
       );
-      await SpecialPowers.spawn(browser, [], async () => {
-        let obj = content.document.querySelector("object");
-        is(
-          obj.displayedType,
-          Ci.nsIObjectLoadingContent.TYPE_DOCUMENT,
-          "should be displaying TYPE_DOCUMENT"
-        );
-      });
     }
   );
+
+  // Clean up our download observer.
+  await downloadList.removeView(downloadView);
+  if (downloadFile) {
+    is(
+      download.source.url,
+      `${httpsTestRoot}/${downloadFile}`,
+      "Download has the correct source"
+    );
+  } else {
+    is(download, undefined, "Should not have seen a download");
+  }
+}
+
+add_task(async function test_pdf_object_attachment() {
+  await SpecialPowers.pushPrefEnv({
+    set: [
+      ["dom.navigation.object_embed.allow_retargeting", false],
+      ["browser.download.open_pdf_attachments_inline", false],
+    ],
+  });
+
+  // PDF attachment should display inline.
+  await loadAndCheck("file_pdf_object_attachment.html", true);
 });
 
 add_task(async function test_img_object_attachment() {
@@ -37,132 +83,71 @@ add_task(async function test_img_object_attachment() {
     set: [["dom.navigation.object_embed.allow_retargeting", false]],
   });
 
-  await BrowserTestUtils.withNewTab(
-    `${httpsTestRoot}/file_img_object_attachment.html`,
-    async browser => {
-      is(
-        browser.browsingContext.children.length,
-        1,
-        "Should have a child frame"
-      );
-      await SpecialPowers.spawn(browser, [], async () => {
-        let obj = content.document.querySelector("object");
-        is(
-          obj.displayedType,
-          Ci.nsIObjectLoadingContent.TYPE_DOCUMENT,
-          "should be displaying TYPE_DOCUMENT"
-        );
-      });
-    }
-  );
+  // Image attachment should display inline.
+  await loadAndCheck("file_img_object_attachment.html", true);
 });
 
-async function waitForDownload() {
-  // Get the downloads list and add a view to listen for a download to be added.
-  let downloadList = await Downloads.getList(Downloads.ALL);
+add_task(async function test_svg_object_attachment() {
+  await SpecialPowers.pushPrefEnv({
+    set: [["dom.navigation.object_embed.allow_retargeting", false]],
+  });
 
-  // Wait for a single download
-  let downloadView;
-  let finishedAllDownloads = new Promise(resolve => {
-    downloadView = {
-      onDownloadAdded(aDownload) {
-        info("download added");
-        resolve(aDownload);
-      },
-    };
+  // SVG attachment should fail to load.
+  await loadAndCheck("file_svg_object_attachment.html", false);
+});
+
+add_task(async function test_html_object_attachment() {
+  await SpecialPowers.pushPrefEnv({
+    set: [["dom.navigation.object_embed.allow_retargeting", false]],
   });
-  await downloadList.addView(downloadView);
-  let download = await finishedAllDownloads;
-  await downloadList.removeView(downloadView);
 
-  // Clean up the download from the list.
-  await downloadList.remove(download);
-  await download.finalize(true);
+  // HTML attachment should fail to load.
+  await loadAndCheck("file_html_object_attachment.html", false);
+});
 
-  // Return the download
-  return download;
-}
+add_task(async function test_pdf_object_attachment_allow_retargeting() {
+  await SpecialPowers.pushPrefEnv({
+    set: [
+      ["dom.navigation.object_embed.allow_retargeting", true],
+      ["browser.download.open_pdf_attachments_inline", false],
+    ],
+  });
+
+  // Even if `allow_retargeting` is enabled, we always display PDFs inline.
+  await loadAndCheck("file_pdf_object_attachment.html", true);
+});
 
-add_task(async function test_pdf_object_attachment_download() {
+add_task(async function test_img_object_attachment_allow_retargeting() {
   await SpecialPowers.pushPrefEnv({
     set: [["dom.navigation.object_embed.allow_retargeting", true]],
   });
 
-  // Set the behaviour to save pdfs to disk and not handle internally, so we
-  // don't end up with extra tabs after the test.
-  var gMimeSvc = Cc["@mozilla.org/mime;1"].getService(Ci.nsIMIMEService);
-  var gHandlerSvc = Cc["@mozilla.org/uriloader/handler-service;1"].getService(
-    Ci.nsIHandlerService
-  );
-  const mimeInfo = gMimeSvc.getFromTypeAndExtension("application/pdf", "pdf");
-  let previousAction = mimeInfo.preferredAction;
-  mimeInfo.preferredAction = Ci.nsIHandlerInfo.saveToDisk;
-  gHandlerSvc.store(mimeInfo);
-  registerCleanupFunction(() => {
-    mimeInfo.preferredAction = previousAction;
-    gHandlerSvc.store(mimeInfo);
-  });
+  // Even if `allow_retargeting` is enabled, we always display images inline.
+  await loadAndCheck("file_img_object_attachment.html", true);
+});
 
-  // Start listening for the download before opening the new tab.
-  let downloadPromise = waitForDownload();
-  await BrowserTestUtils.withNewTab(
-    `${httpsTestRoot}/file_pdf_object_attachment.html`,
-    async browser => {
-      let download = await downloadPromise;
-      is(
-        download.source.url,
-        `${httpsTestRoot}/file_pdf_attachment.pdf`,
-        "download should be the pdf"
-      );
+add_task(async function test_svg_object_attachment_allow_retargeting() {
+  await SpecialPowers.pushPrefEnv({
+    set: [["dom.navigation.object_embed.allow_retargeting", true]],
+  });
 
-      await SpecialPowers.spawn(browser, [], async () => {
-        let obj = content.document.querySelector("object");
-        is(
-          obj.displayedType,
-          Ci.nsIObjectLoadingContent.TYPE_FALLBACK,
-          "should be displaying TYPE_FALLBACK"
-        );
-      });
-    }
+  // SVG attachments are downloaded if allow_retargeting is set.
+  await loadAndCheck(
+    "file_svg_object_attachment.html",
+    false,
+    "file_svg_attachment.svg"
   );
 });
 
-add_task(async function test_img_object_attachment_download() {
-  // NOTE: This is testing our current behaviour here as of bug 1868001 (which
-  // is to download an image with `Content-Disposition: attachment` embedded
-  // within an object or embed element).
-  //
-  // Other browsers ignore the `Content-Disposition: attachment` header when
-  // loading images within object or embed element as-of december 2023, as
-  // we did prior to the changes in bug 1595491.
-  //
-  // If this turns out to be a web-compat issue, we may want to introduce
-  // special handling to ignore content-disposition when loading images within
-  // an object or embed element.
+add_task(async function test_html_object_attachment_allow_retargeting() {
   await SpecialPowers.pushPrefEnv({
     set: [["dom.navigation.object_embed.allow_retargeting", true]],
   });
 
-  // Start listening for the download before opening the new tab.
-  let downloadPromise = waitForDownload();
-  await BrowserTestUtils.withNewTab(
-    `${httpsTestRoot}/file_img_object_attachment.html`,
-    async browser => {
-      let download = await downloadPromise;
-      is(
-        download.source.url,
-        `${httpsTestRoot}/file_img_attachment.jpg`,
-        "download should be the jpg"
-      );
-
-      await SpecialPowers.spawn(browser, [], async () => {
-        let obj = content.document.querySelector("object");
-        is(
-          obj.displayedType,
-          Ci.nsIObjectLoadingContent.TYPE_FALLBACK,
-          "should be displaying TYPE_FALLBACK"
-        );
-      });
-    }
+  // HTML attachments are downloaded if allow_retargeting is set.
+  await loadAndCheck(
+    "file_html_object_attachment.html",
+    false,
+    "file_html_attachment.html"
   );
 });
diff --git a/dom/base/test/file_html_attachment.html b/dom/base/test/file_html_attachment.html
new file mode 100644
index 00000000000..f23eccfecf4
--- /dev/null
+++ b/dom/base/test/file_html_attachment.html
@@ -0,0 +1,6 @@
+<!DOCTYPE html>
+<html>
+  <body>
... (truncated)
Loading diff…