Low watch CVSS 5.4 chrome Logic Error 📄 Reporter bug report 🔧 Commit mapped

Overview

Low
Severity
5.4
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactInappropriate implementation in Navigations
DescriptionInappropriate implementation in Navigations
ComponentNavigations
Sub-componentInternals>Sandbox>SiteIsolation, UI>Browser>Navigation
Bug ClassLogic Error
Action tierwatch — logic/policy bug — security-relevant, no memory-unsafety
Computed by PatchGap's deterministic exploitability ruler from the fixing commit. patch-now = RCE-class & attacker-reachable in a default config, or exploited in the wild; watch = real security signal below that bar; noise = no security relevance.
Tracker40051596
Fix commitcdb798347ad7 (chromium/src) +109/-22
CWECWE-346 (Origin validation error)
CVSS vectorCVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:U/C:L/I:L/A:N
CISA KEVNot listed
CreditedDavid Erceg
Reported2020-02-23 · bounty $2000 (per vendor advisory)
Fix landed2025-02-14 (public git)
Disclosed2025-04-01
Patch gap46 days — fix public in source before the advisory

Background

Document sequence number (DSN)
a per-FrameNavigationEntry identifier used by session history to decide whether two navigations refer to the “same document,” so that navigating between two entries with the same DSN is treated as a same-document navigation.
`FrameNavigationEntry`
the per-frame history state (URL, origin, DSN, history.state) stored in a NavigationEntry, retrieved via GetFrameEntry.
Opaque origin precursor
the tuple recorded via GetTupleOrPrecursorTupleIfOpaque() that captures which real origin an opaque origin (such as an about:blank document or an error page) descended from.
`CommitNavigationParams`
the browser-to-renderer struct describing a commit; the fix adds a force_new_document_sequence_number field so the browser can instruct the renderer not to reuse the DSN.

Root Cause Analysis

Before the patch, cross-document navigations reused the document sequence number for “logically related” navigations, and the renderer made this decision without being told about cross-origin transitions. The removed test assertion EXPECT_EQ(dsn_a1, dsn_blank) (with its TODO(crbug.com/40051596)) documents the wrong behavior: navigating a frame to about:blank under a different owner kept the same DSN as the prior origin a1. Because two history entries that share a DSN are treated as the same document, a later cross-origin navigation could later be misclassified as a same-document navigation, breaking the invariant that same-document navigations stay within one security origin.

The fix computes the cross-origin condition in the browser process (navigation_request.cc) and sets commit_params_->force_new_document_sequence_number = true so the renderer allocates a fresh DSN, with the new assertion changed to EXPECT_NE(dsn_a1, dsn_blank). An exception is carved out for compatible error-page transitions (matching valid precursor tuples), so temporary failure/retry flows preserve history item state including history.state (EXPECT_EQ("foo", ...)).

Key insight
The core mistake was letting the renderer reuse a document sequence number across a cross-origin boundary, which lets session history conflate two different-origin documents as one; the fix moves the decision to the browser and forces a new DSN for cross-origin navigations, except for compatible error-page state-preservation transitions.

Attack Path

  1. Create a frame with a real origin A page loads a subframe (e.g., origin a1) whose FrameNavigationEntry records a document sequence number dsn_a1.
  2. Navigate cross-origin with reuse The frame navigates to a cross-origin document such as about:blank under a different owner; pre-patch, the renderer reuses dsn_a1 for the new entry instead of assigning a distinct DSN.
  3. DSN collision across origins Two history entries now share a document sequence number despite belonging to different origins, violating the same-document invariant.
  4. Trigger misclassification A subsequent history/back or forward navigation between the colliding entries is evaluated as same-document because the DSNs match, so a cross-origin transition is handled on the same-document path (inferred), enabling the “inappropriate implementation in Navigations” behavior the advisory describes.

Impact Assessment

An attacker-controlled page can cause the browser to misclassify a cross-origin navigation as same-document, producing incorrect session-history and origin handling in the renderer’s navigation logic. The precondition is the ability to script frame navigations across an origin boundary (e.g., to about:blank with a different owner) within a page the attacker controls. The advisory classifies this as a LogicError / inappropriate implementation in Navigations and does not report exploitation in the wild; any escalation to a concrete cross-origin data disclosure or spoofing primitive is (inferred).

Changed Functions

FunctionChangeNotes
if
content/browser/renderer_host/navigation_request.cc
modified
if
content/browser/renderer_host/render_frame_host_impl.cc
modified

Files Changed

  • content/browser/renderer_host/navigation_controller_impl.cc
  • content/browser/renderer_host/navigation_controller_impl_browsertest.cc
  • content/browser/renderer_host/navigation_entry_impl.cc
  • content/browser/renderer_host/navigation_request.cc
  • content/browser/renderer_host/render_frame_host_impl.cc

Audit Directions

  • Renderer-trusted history decisions
    Look for other places where the renderer, not the browser, decides same-document vs. cross-document status or reuses history identifiers (DSN, item sequence number) across origin boundaries.
  • Opaque-origin precursor handling
    Audit code that compares GetTupleOrPrecursorTupleIfOpaque() for about:blank, sandboxed frames, and error pages, where an opaque origin can carry a precursor that silently equates two distinct security contexts.
  • Error-page state preservation exceptions
    Review the error-page carve-outs that preserve history item state; ensure the precursor-matching exception cannot be abused to preserve or reuse DSNs across genuinely cross-origin transitions.
From cdb798347ad78bcb1cffdbdc0e06eb1ec1867a57 Mon Sep 17 00:00:00 2001
From: Charlie Reis <creis@chromium.org>
Date: Fri, 14 Feb 2025 14:04:46 -0800
Subject: [PATCH] Do not reuse document sequence number on cross-origin navigations.

Cross-document navigations only reuse document sequence numbers for
"logically related" navigations. This CL ensures that they are not
reused for cross-origin navigations as well (e.g., about:blank with
different owners), by computing this in the browser process and sending
it in the CommitNavigationParams.

Note that an exception is needed for error pages, which should preserve
the document sequence number and other history item state, though they
transition to and from an opaque origin with the same precursor.

Bug: 40051596
Change-Id: I502251ce12ec8b3e613596b914fbe8a63330b4fc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6185347
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1420736}
---

diff --git a/content/browser/renderer_host/navigation_controller_impl.cc b/content/browser/renderer_host/navigation_controller_impl.cc
index 597384a..715a22a 100644
--- a/content/browser/renderer_host/navigation_controller_impl.cc
+++ b/content/browser/renderer_host/navigation_controller_impl.cc
@@ -4125,7 +4125,8 @@
           /*visited_link_salt=*/std::nullopt,
           /*local_surface_id=*/std::nullopt,
           node->current_frame_host()->GetCachedPermissionStatuses(),
-          /*should_skip_screentshot=*/false);
+          /*should_skip_screentshot=*/false,
+          /*force_new_document_sequence_number=*/false);
 #if BUILDFLAG(IS_ANDROID)
   if (ValidateDataURLAsString(params.data_url_as_string)) {
     commit_params->data_url_as_string = params.data_url_as_string->as_string();
diff --git a/content/browser/renderer_host/navigation_controller_impl_browsertest.cc b/content/browser/renderer_host/navigation_controller_impl_browsertest.cc
index 7923c93..9a7f5fe60 100644
--- a/content/browser/renderer_host/navigation_controller_impl_browsertest.cc
+++ b/content/browser/renderer_host/navigation_controller_impl_browsertest.cc
@@ -5304,9 +5304,7 @@
   scoped_refptr<FrameNavigationEntry> frame_entry_blank_data =
       controller.GetLastCommittedEntry()->GetFrameEntry(inner_frame);
   int64_t dsn_blank = frame_entry_blank_data->document_sequence_number();
-  //  TODO(crbug.com/40051596): Fix Blink to use a different document sequence
-  //  number for this navigation.
-  EXPECT_EQ(dsn_a1, dsn_blank);
+  EXPECT_NE(dsn_a1, dsn_blank);
 
   // Go back. This should not be treated as same-document, because the origin
   // changed in the previous navigation.
@@ -19169,6 +19167,10 @@
     EXPECT_EQ(previous_frame_entry,
               controller.GetLastCommittedEntry()->GetFrameEntry(child));
     EXPECT_TRUE(capturer.did_replace_entry());
+
+    // We keep the same history.state value, even in the error page, so that it
+    // can be used when the load later succeeds in step 4.
+    EXPECT_EQ("foo", EvalJs(child, "history.state"));
   }
 
   // 4) Test successfully navigating the subframe to the same URL after a failed
diff --git a/content/browser/renderer_host/navigation_entry_impl.cc b/content/browser/renderer_host/navigation_entry_impl.cc
index c1d1d97d..5578f71 100644
--- a/content/browser/renderer_host/navigation_entry_impl.cc
+++ b/content/browser/renderer_host/navigation_entry_impl.cc
@@ -1005,7 +1005,8 @@
           /*visited_link_salt=*/std::nullopt,
           /*local_surface_id=*/std::nullopt,
           /*initial_permission_statuses=*/std::nullopt,
-          /*should_skip_screenshot*/ false);
+          /*should_skip_screenshot*/ false,
+          /*force_new_document_sequence_number=*/false);
 #if BUILDFLAG(IS_ANDROID)
   // `data_url_as_string` is saved in NavigationEntry but should only be used by
   // main frames, because loadData* navigations can only happen on the main
diff --git a/content/browser/renderer_host/navigation_request.cc b/content/browser/renderer_host/navigation_request.cc
index 4921db5..07309925 100644
--- a/content/browser/renderer_host/navigation_request.cc
+++ b/content/browser/renderer_host/navigation_request.cc
@@ -1445,7 +1445,8 @@
           /*visited_link_salt=*/std::nullopt,
           /*local_surface_id=*/std::nullopt,
           frame_tree_node->current_frame_host()->GetCachedPermissionStatuses(),
-          /*should_skip_screenshot=*/false);
+          /*should_skip_screenshot=*/false,
+          /*force_new_document_sequence_number=*/false);
 
   commit_params->navigation_timing->system_entropy_at_navigation_start =
       SystemEntropyUtils::ComputeSystemEntropyForFrameTreeNode(
@@ -1597,7 +1598,8 @@
           /*visited_link_salt=*/std::nullopt,
           /*local_surface_id=*/std::nullopt,
           render_frame_host->GetCachedPermissionStatuses(),
-          /*should_skip_screenshot=*/false);
+          /*should_skip_screenshot=*/false,
+          /*force_new_document_sequence_number=*/false);
   blink::mojom::BeginNavigationParamsPtr begin_params =
       blink::mojom::BeginNavigationParams::New();
   std::unique_ptr<NavigationRequest> navigation_request(new NavigationRequest(
@@ -6061,6 +6063,30 @@
     return;
   }
 
+  // Ensure the renderer does not reuse the document sequence number for
+  // cross-origin navigations (which can lead to later bugs with same-document
+  // navigations appearing to be cross-origin). All error pages have unique
+  // opaque origins and are considered cross-origin.
+  //
+  // The one exception is for transitioning to or from a compatible error page,
+  // which preserves state in case a temporary failure later succeeds. Here, we
+  // must check for any cases where the committing error page has a valid
+  // precursor that agrees with the current document, whether the current
+  // document is already an error page or not. (The renderer process will narrow
+  // DSN reuse further, to cases the URL is a closer match, ignoring fragments.)
+  // See also CommitNavigation for the other direction.
+  // TODO(crbug.com/396645697): Use a different technique for preserving history
+  // item state on error pages, separate from the error page's own state.
+  RenderFrameHostImpl* previous_rfh = frame_tree_node()->current_frame_host();
+  const url::Origin& previous_origin = previous_rfh->GetLastCommittedOrigin();
+  bool is_error_page_with_same_precursor =
+      previous_origin.GetTupleOrPrecursorTupleIfOpaque().IsValid() &&
+      commit_params_->origin_to_commit->GetTupleOrPrecursorTupleIfOpaque() ==
+          previous_origin.GetTupleOrPrecursorTupleIfOpaque();
+  if (!is_error_page_with_same_precursor) {
+    commit_params_->force_new_document_sequence_number = true;
+  }
+
   PopulateDocumentTokenForCrossDocumentNavigation();
   // Use a separate cache shard, and no cookies, for error pages.
   isolation_info_for_subresources_ =
@@ -6375,6 +6401,35 @@
         GetNavigationController()->GetNavigationApiHistoryEntryVectors(
             frame_tree_node_, this);
     PopulateDocumentTokenForCrossDocumentNavigation();
+
+    // Ensure the renderer does not reuse the document sequence number for
+    // cross-origin navigations (which can lead to later bugs with same-document
+    // navigations appearing to be cross-origin).
+    //
+    // The one exception is for transitioning to or from a compatible error
+    // page, which preserves state in case a temporary failure later succeeds.
+    // Here, we must check for the case that the user navigates from an error
+    // page to a non-error page that matches the (valid) precursor origin. See
+    // also CommitErrorPage for the other direction.
+    // TODO(crbug.com/396645697): Use a different technique for preserving
+    // history item state on error pages, separate from the error page's own
+    // state.
+    RenderFrameHostImpl* previous_rfh = frame_tree_node()->current_frame_host();
+    const url::Origin& previous_origin = previous_rfh->GetLastCommittedOrigin();
+    // Skip this check if kUseBrowserCalculatedOrigin is disabled.
+    if (base::FeatureList::IsEnabled(features::kUseBrowserCalculatedOrigin)) {
+      bool is_cross_origin_navigation =
+          !commit_params_->origin_to_commit->IsSameOriginWith(previous_origin);
+      bool compatible_with_error_page =
+          previous_rfh->IsErrorDocument() &&
+          previous_origin.GetTupleOrPrecursorTupleIfOpaque().IsValid() &&
+          commit_params_->origin_to_commit
+                  ->GetTupleOrPrecursorTupleIfOpaque() ==
+              previous_origin.GetTupleOrPrecursorTupleIfOpaque();
+      if (is_cross_origin_navigation && !compatible_with_error_page) {
+        commit_params_->force_new_document_sequence_number = true;
+      }
+    }
   }
 
   if (early_hints_manager_) {
diff --git a/content/browser/renderer_host/render_frame_host_impl.cc b/content/browser/renderer_host/render_frame_host_impl.cc
index 19601b38..69e121b0 100644
--- a/content/browser/renderer_host/render_frame_host_impl.cc
+++ b/content/browser/renderer_host/render_frame_host_impl.cc
@@ -12227,9 +12227,10 @@
   commit_params->should_skip_screenshot =
       NavigationTransitionUtils::ShouldSkipScreenshot(*navigation_request);
 
+  RenderFrameHostImpl* previous_rfh =
+      navigation_request->frame_tree_node()->current_frame_host();
   if (is_same_document) {
-    DCHECK_EQ(navigation_request->frame_tree_node()->current_frame_host(),
-              this);
+    DCHECK_EQ(previous_rfh, this);
     const base::UnguessableToken& navigation_token =
         commit_params->navigation_token;
     commit_params->has_ua_visual_transition =
@@ -12358,8 +12359,7 @@
     // point just before the navigation commits.
     // TODO(altimin, crbug.com/933147): Remove this logic after we are done with
     // implementing back-forward cache.
-    if (!GetParent() &&
-        navigation_request->frame_tree_node()->current_frame_host() == this) {
+    if (!GetParent() && previous_rfh == this) {
       if (NavigationEntryImpl* last_committed_entry =
               NavigationEntryImpl::FromNavigationEntry(
                   navigation_request->frame_tree_node()
@@ -12390,10 +12390,7 @@
     // processes.
     const bool maybe_new_process_is_used =
         GetProcess()->GetRenderFrameHostCount() == 1 &&
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/content/browser/renderer_host/navigation_controller_impl_browsertest.cc b/content/browser/renderer_host/navigation_controller_impl_browsertest.cc
index 7923c93..9a7f5fe60 100644
--- a/content/browser/renderer_host/navigation_controller_impl_browsertest.cc
+++ b/content/browser/renderer_host/navigation_controller_impl_browsertest.cc
@@ -5304,9 +5304,7 @@
   scoped_refptr<FrameNavigationEntry> frame_entry_blank_data =
       controller.GetLastCommittedEntry()->GetFrameEntry(inner_frame);
   int64_t dsn_blank = frame_entry_blank_data->document_sequence_number();
-  //  TODO(crbug.com/40051596): Fix Blink to use a different document sequence
-  //  number for this navigation.
-  EXPECT_EQ(dsn_a1, dsn_blank);
+  EXPECT_NE(dsn_a1, dsn_blank);
 
   // Go back. This should not be treated as same-document, because the origin
   // changed in the previous navigation.
@@ -19169,6 +19167,10 @@
     EXPECT_EQ(previous_frame_entry,
               controller.GetLastCommittedEntry()->GetFrameEntry(child));
     EXPECT_TRUE(capturer.did_replace_entry());
+
+    // We keep the same history.state value, even in the error page, so that it
+    // can be used when the load later succeeds in step 4.
+    EXPECT_EQ("foo", EvalJs(child, "history.state"));
   }
 
   // 4) Test successfully navigating the subframe to the same URL after a failed
Loading diff…

Original Bug Report

reported by de...@gmail.com

Security: Possible to cause incorrect origin to be used when performing a same document navigation

VULNERABILITY DETAILS
It’s possible for a window to perform a same document navigation between its original URL (e.g. a http/https URL) and about:blank. If another window within the same namespace navigates the first window to about:blank and then goes back, a same document navigation will be performed, though the origin will now have changed to that of the second window.

This allows, for example, a data: window to control another window whose origin is opaque (the same opaque origin as the data: window), but whose visible URL is the original (http/https) URL.

VERSION
Chrome Version: Tested on 80.0.3987.116 (stable) and 82.0.4067.0 (canary)
Operating System: Windows 10, version 1909

REPRODUCTION CASE

  1. Download the attached files into a directory, then run the following command:

python3 -m http.server 8080

  1. In the browser, navigate to the following location:

http://localhost:8080/index.html

  1. Wait three seconds.
  2. index.html contains two iframes: a data: iframe and an iframe pointing to http://localhost:8080/iframe.html nested within it.

Click within the innermost iframe. This should open a new window with a visible URL of http://localhost:8080/iframe.html and an opaque origin (that matches the origin of the data: iframe on the original page).

The data: iframe controls this window. To demonstrate this, it adds the following content to the new page:

Content set by data: iframe

This seemingly shouldn’t be possible, as the data: frame has an opaque origin, one that’s not the same as the origin represented by a http://localhost:8080 URL.

CREDIT INFORMATION
Reporter credit: David Erceg

View on issue tracker