Low firefox Logic Error 🔧 Commit mapped

Overview

Low
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactlow
DescriptionSpoofing issue in the DOM: Navigation component
ComponentCore
Bug ClassLogic Error
Tracker2059196
Fix commit5f6d0d470030 (firefox) +103/-11
CISA KEVNot listed
CreditedMozilla
Disclosed2026-09-15

Changed Functions

FunctionChangeNotes
if
docshell/base/nsDocShell.cpp
modified

Files Changed

  • docshell/base/nsDocShell.cpp
  • testing/web-platform/tests/navigation-api/precommit-handler/precommitHandler-javascript-url-before-commit.html
diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp
index 4235b183aa8..06c3c9b8be2 100644
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -8721,13 +8721,31 @@ nsresult nsDocShell::InternalLoad(nsDocShellLoadState* aLoadState,
   }
 
   // The following steps are from https://html.spec.whatwg.org/#navigate
-  // Step 19, and here we actually also perform step 2 from
-  // #navigate-to-a-javascript:-url (step 20) where the ongoing navigation is
-  // set to null.
-  SetOngoingNavigation(isJavaScript ? Nothing()
-                                    : Some(OngoingNavigation::NavigationID));
+  // 20. Set the ongoing navigation for navigable to navigationId.
+  SetOngoingNavigation(Some(OngoingNavigation::NavigationID));
+
+  // 21. If url's scheme is "javascript":
+  if (isJavaScript) {
+    // 21.1 Queue a global task on the navigation and traversal task source
+    //      given navigable's active window to navigate to a javascript: URL
+    //      given navigable, url, historyHandling, sourceSnapshotParams,
+    //      initiatorOriginSnapshot, userInvolvement, cspNavigationType,
+    //      initialInsertion, and navigationId.
+    // 21.2 Return.
+    //
+    // Note: We don't queue a task, so instead we run step 3 of
+    // https://html.spec.whatwg.org/#navigate-to-a-javascript:-url
+    // 3. Set the ongoing navigation for targetNavigable to null.
+    SetOngoingNavigation(Nothing());
+  }
 
-  // Step 21
+  // 22. If all of the following are true:
+  //    * userInvolvement is not "browser UI";
+  //    * navigable's active document's origin is same origin-domain with
+  //      sourceDocument's origin;
+  //    * navigable's active document's is initial about:blank is false; and
+  //    * url's scheme is a fetch scheme,
+  //    then:
   if (RefPtr<Document> document = GetDocument();
       !aLoadState->LoadIsFromSessionHistory() && document &&
       aLoadState->UserNavigationInvolvement() !=
@@ -8738,21 +8756,31 @@ nsresult nsDocShell::InternalLoad(nsDocShellLoadState* aLoadState,
       document->NodePrincipal()->EqualsConsideringDomain(
           aLoadState->TriggeringPrincipal())) {
     if (nsCOMPtr<nsPIDOMWindowInner> window = document->GetInnerWindow()) {
-      // Step 21.1
+      // 22.1 Let navigation be navigable's active window's navigation API.
       if (RefPtr<Navigation> navigation = window->Navigation()) {
         AutoJSAPI jsapi;
         if (jsapi.Init(window)) {
           RefPtr<Element> sourceElement = aLoadState->GetSourceElement();
 
-          // Step 21.2
+          // 22.2 Let entryListForFiring be formDataEntryList if
+          //      documentResource is a POST resource; otherwise, null.
           RefPtr<FormData> formData = aLoadState->GetFormDataEntryList();
 
-          // Step 21.3
+          // 22.3 Let navigationAPIStateForFiring be navigationAPIState if
+          //      navigationAPIState is not null; otherwise,
+          //      StructuredSerializeForStorage(undefined).
           RefPtr<nsIStructuredCloneContainer> navigationAPIStateForFiring =
               aLoadState->GetNavigationAPIState();
 
           nsCOMPtr<nsIURI> destinationURL = aLoadState->URI();
-          // Step 21.4
+          // 22.4 Let continue be the result of firing a push/replace/reload
+          //      navigate event at navigation with navigationType set to
+          //      historyHandling, isSameDocument set to false, userInvolvement
+          //      set to userInvolvement, sourceElement set to sourceElement,
+          //      formDataEntryList set to entryListForFiring, destinationURL
+          //      set to url, navigationAPIState set to
+          //      navigationAPIStateForFiring, and apiMethodTracker set to
+          //      apiMethodTracker.
           RefPtr apiMethodTracker = aLoadState->GetNavigationAPIMethodTracker();
           bool shouldContinue = navigation->FirePushReplaceReloadNavigateEvent(
               jsapi.cx(), aLoadState->GetNavigationType(), destinationURL,
@@ -8761,7 +8789,7 @@ nsresult nsDocShell::InternalLoad(nsDocShellLoadState* aLoadState,
               formData, navigationAPIStateForFiring,
               /* aClassicHistoryAPIState */ nullptr, apiMethodTracker);
 
-          // Step 21.5
+          // 22.5 If continue is false, then return.
           if (!shouldContinue) {
             return NS_OK;
           }
diff --git a/testing/web-platform/tests/navigation-api/precommit-handler/precommitHandler-javascript-url-before-commit.html b/testing/web-platform/tests/navigation-api/precommit-handler/precommitHandler-javascript-url-before-commit.html
new file mode 100644
index 00000000000..f0d58b13707
--- /dev/null
+++ b/testing/web-platform/tests/navigation-api/precommit-handler/precommitHandler-javascript-url-before-commit.html
@@ -0,0 +1,64 @@
+<!doctype html>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<link rel="help" href="https://html.spec.whatwg.org/#navigate" />
+<link rel="help" href="https://html.spec.whatwg.org/#navigate-to-a-javascript:-url"/>
+<link rel="help" href="https://html.spec.whatwg.org/#set-the-ongoing-navigation"/>
+<body>
+  <script>
+    // A navigate event that is intercepted with a precommitHandler stays
+    // ongoing for as long as that handler's promise is pending: the URL doesn't
+    // change and the navigation isn't committed. Starting a second navigation
+    // while the first one is parked like this has to abort the parked one.
+    //
+    // #navigate step 20 ("Set the ongoing navigation for navigable to
+    // navigationId.") runs for every scheme, and a changed value makes
+    // #set-the-ongoing-navigation step 2 abort whatever was ongoing. Only
+    // afterwards does #navigate step 21 queue the task that evaluates a
+    // javascript: URL and replaces the document. So the parked navigate event
+    // must already be aborted by the time the old document is unloaded.
+    //
+    // This test parks a fragment navigation in its precommitHandler, navigates
+    // the same window to a javascript: URL, and checks at pagehide that the
+    // parked event was aborted.
+    promise_test(async t => {
+      const iframe = document.createElement("iframe");
+      iframe.src = "/common/blank.html";
+      document.body.appendChild(iframe);
+      await new Promise(resolve => (iframe.onload = resolve));
+
+      const win = iframe.contentWindow;
+      let signal;
+      let precommitHandlerReached;
+      const reached = new Promise(
+        resolve => (precommitHandlerReached = resolve)
+      );
+      win.navigation.addEventListener(
+        "navigate",
+        e => {
+          signal = e.signal;
+          e.intercept({
+            precommitHandler: () => {
+              precommitHandlerReached();
+              // Never resolves, so the navigate event stays ongoing.
+              return new Promise(() => {});
+            },
+          });
+        },
+        { once: true }
+      );
+
+      win.location.hash = "#ShouldNotCommit";
+      await reached;
+      assert_false(signal.aborted, "aborted before the javascript: URL");
+
+      const abortedAtPagehide = new Promise(resolve =>
+        win.addEventListener("pagehide", () => resolve(signal.aborted), {
+          once: true,
+        })
+      );
+      win.location.href = "javascript:'new document'";
+      assert_true(await abortedAtPagehide, "aborted before pagehide");
+    }, "A javascript: URL navigation aborts a navigate event whose precommitHandler is still pending");
+  </script>
+</body>
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/testing/web-platform/tests/navigation-api/precommit-handler/precommitHandler-javascript-url-before-commit.html b/testing/web-platform/tests/navigation-api/precommit-handler/precommitHandler-javascript-url-before-commit.html
new file mode 100644
index 00000000000..f0d58b13707
--- /dev/null
+++ b/testing/web-platform/tests/navigation-api/precommit-handler/precommitHandler-javascript-url-before-commit.html
@@ -0,0 +1,64 @@
+<!doctype html>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<link rel="help" href="https://html.spec.whatwg.org/#navigate" />
+<link rel="help" href="https://html.spec.whatwg.org/#navigate-to-a-javascript:-url"/>
+<link rel="help" href="https://html.spec.whatwg.org/#set-the-ongoing-navigation"/>
+<body>
+  <script>
+    // A navigate event that is intercepted with a precommitHandler stays
+    // ongoing for as long as that handler's promise is pending: the URL doesn't
+    // change and the navigation isn't committed. Starting a second navigation
+    // while the first one is parked like this has to abort the parked one.
+    //
+    // #navigate step 20 ("Set the ongoing navigation for navigable to
+    // navigationId.") runs for every scheme, and a changed value makes
+    // #set-the-ongoing-navigation step 2 abort whatever was ongoing. Only
+    // afterwards does #navigate step 21 queue the task that evaluates a
+    // javascript: URL and replaces the document. So the parked navigate event
+    // must already be aborted by the time the old document is unloaded.
+    //
+    // This test parks a fragment navigation in its precommitHandler, navigates
+    // the same window to a javascript: URL, and checks at pagehide that the
+    // parked event was aborted.
+    promise_test(async t => {
+      const iframe = document.createElement("iframe");
+      iframe.src = "/common/blank.html";
+      document.body.appendChild(iframe);
+      await new Promise(resolve => (iframe.onload = resolve));
+
+      const win = iframe.contentWindow;
+      let signal;
+      let precommitHandlerReached;
+      const reached = new Promise(
+        resolve => (precommitHandlerReached = resolve)
+      );
+      win.navigation.addEventListener(
+        "navigate",
+        e => {
+          signal = e.signal;
+          e.intercept({
+            precommitHandler: () => {
+              precommitHandlerReached();
+              // Never resolves, so the navigate event stays ongoing.
+              return new Promise(() => {});
+            },
+          });
+        },
+        { once: true }
+      );
+
+      win.location.hash = "#ShouldNotCommit";
+      await reached;
+      assert_false(signal.aborted, "aborted before the javascript: URL");
+
+      const abortedAtPagehide = new Promise(resolve =>
+        win.addEventListener("pagehide", () => resolve(signal.aborted), {
+          once: true,
+        })
+      );
+      win.location.href = "javascript:'new document'";
+      assert_true(await abortedAtPagehide, "aborted before pagehide");
+    }, "A javascript: URL navigation aborts a navigate event whose precommitHandler is still pending");
+  </script>
+</body>
Loading diff…