From 03580574961d57c7ba5723afdc6605045b4fa0ef Mon Sep 17 00:00:00 2001
From: Charlie Reis <[email protected]>
Date: Thu, 26 Feb 2026 15:23:48 -0800
Subject: [PATCH] Ensure that all files in a PageState are present in GetReferencedFiles.

This depends on updating AppendReferencedFilesFromDocumentState to
handle PageState version 14, which can have value_sizes of 0 or
multiples of 3.

Bug: 487383169
Change-Id: I3c8d3ee7d198d7fad5d180cf4e4e8bd62d18d79b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7609011
Reviewed-by: Kent Tamura <[email protected]>
Reviewed-by: Alex Moshchuk <[email protected]>
Commit-Queue: Charlie Reis <[email protected]>
Auto-Submit: Charlie Reis <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1591151}
---

diff --git a/content/browser/renderer_host/render_frame_host_impl.cc b/content/browser/renderer_host/render_frame_host_impl.cc
index 2513348..a71222b 100644
--- a/content/browser/renderer_host/render_frame_host_impl.cc
+++ b/content/browser/renderer_host/render_frame_host_impl.cc
@@ -279,6 +279,7 @@
 #include "third_party/blink/public/common/loader/resource_type_util.h"
 #include "third_party/blink/public/common/messaging/transferable_message.h"
 #include "third_party/blink/public/common/navigation/navigation_params_mojom_traits.h"
+#include "third_party/blink/public/common/page_state/page_state_serialization.h"
 #include "third_party/blink/public/common/permissions/permission_utils.h"
 #include "third_party/blink/public/common/permissions_policy/document_policy.h"
 #include "third_party/blink/public/common/permissions_policy/policy_helper_public.h"
@@ -13729,8 +13730,29 @@
 
 bool RenderFrameHostImpl::CanAccessFilesOfPageState(
     const blink::PageState& state) {
+  // Ensure that all of the files in the PageState were actually listed in the
+  // GetReferencedFiles list, using a set to prune duplicates.
+  // See https://crbug.com/487383169.
+  std::vector<base::FilePath> all_files;
+  if (!blink::GetAllFilesInPageState(state.ToEncodedData(), &all_files)) {
+    // All files in the PageState weren't recovered due to parsing failures.
+    // The renderer should be killed instead of proceeding with a PageState that
+    // might still contain files that could be used without being validated.
+    return false;
+  }
+  std::vector<base::FilePath> referenced_files = state.GetReferencedFiles();
+  std::set<base::FilePath> referenced_file_set(referenced_files.begin(),
+                                               referenced_files.end());
+  for (const base::FilePath& file : all_files) {
+    if (!referenced_file_set.contains(file)) {
+      // Found a file that was not in the list to be validated, so the renderer
+      // should be killed.
+      return false;
+    }
+  }
+
   return ChildProcessSecurityPolicyImpl::GetInstance()->CanReadAllFiles(
-      GetProcess()->GetID(), state.GetReferencedFiles());
+      GetProcess()->GetID(), referenced_files);
 }
 
 void RenderFrameHostImpl::GrantFileAccessFromPageState(
diff --git a/content/browser/security_exploit_browsertest.cc b/content/browser/security_exploit_browsertest.cc
index a0d347f..4d0c4617 100644
--- a/content/browser/security_exploit_browsertest.cc
+++ b/content/browser/security_exploit_browsertest.cc
@@ -90,6 +90,7 @@
 #include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
 #include "services/network/public/cpp/network_switches.h"
 #include "services/network/public/cpp/resource_request.h"
+#include "services/network/public/cpp/resource_request_body.h"
 #include "services/network/public/mojom/fetch_api.mojom.h"
 #include "services/network/public/mojom/trust_tokens.mojom.h"
 #include "services/network/public/mojom/url_loader.mojom.h"
@@ -102,6 +103,7 @@
 #include "third_party/blink/public/common/fenced_frame/fenced_frame_utils.h"
 #include "third_party/blink/public/common/frame/fenced_frame_sandbox_flags.h"
 #include "third_party/blink/public/common/navigation/navigation_policy.h"
+#include "third_party/blink/public/common/page_state/page_state_serialization.h"
 #include "third_party/blink/public/mojom/blob/blob_url_store.mojom.h"
 #include "third_party/blink/public/mojom/choosers/file_chooser.mojom.h"
 #include "third_party/blink/public/mojom/fenced_frame/fenced_frame.mojom.h"
@@ -1596,6 +1598,130 @@
   EXPECT_EQ(bad_message::RFH_INVALID_WEB_UI_CONTROLLER, kill_waiter.Wait());
 }
 
+namespace {
+
+// An interceptor class that allows replacing the PageState of the DidCommit IPC
+// from the renderer process to the browser process.
+class DidCommitPageStateReplacer : public DidCommitNavigationInterceptor {
+ public:
+  DidCommitPageStateReplacer(WebContents* web_contents,
+                             const blink::PageState& page_state)
+      : DidCommitNavigationInterceptor(web_contents),
+        replacement_page_state_(page_state) {}
+
+  DidCommitPageStateReplacer(const DidCommitPageStateReplacer&) = delete;
+  DidCommitPageStateReplacer& operator=(const DidCommitPageStateReplacer&) =
+      delete;
+
+  ~DidCommitPageStateReplacer() override = default;
+
+ protected:
+  bool WillProcessDidCommitNavigation(
+      RenderFrameHost* render_frame_host,
+      NavigationRequest* navigation_request,
+      mojom::DidCommitProvisionalLoadParamsPtr* params,
+      mojom::DidCommitProvisionalLoadInterfaceParamsPtr* interface_params)
+      override {
+    (**params).page_state = replacement_page_state_;
+    return true;
+  }
+
+ private:
+  blink::PageState replacement_page_state_;
+};
+
+}  // namespace
+
+// Test that committing a navigation with a PageState that does not list all of
+// its file paths in GetReferencedFiles will cause a renderer kill.
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest, PageStateWithUnlistedFile) {
+  // Navigate to foo.com initially.
+  GURL foo_url(embedded_test_server()->GetURL("foo.com", "/title1.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), foo_url));
+
+  // Create a PageState that contains a file path which isn't in the list of
+  // referenced files which are validated.
+  GURL foo_url2(embedded_test_server()->GetURL("foo.com", "/title2.html"));
+  blink::ExplodedPageState exploded_page_state;
+  ASSERT_TRUE(blink::DecodePageState(
+      blink::PageState::CreateFromURL(foo_url2).ToEncodedData(),
+      &exploded_page_state));
+  scoped_refptr<network::ResourceRequestBody> request_body =
+      new network::ResourceRequestBody();
+  base::FilePath bad_file = base::FilePath::FromUTF8Unsafe("/tmp/offlimits");
+  request_body->AppendFileRange(
+      bad_file, 0, std::numeric_limits<uint64_t>::max(), base::Time());
+  exploded_page_state.top.http_body.request_body = request_body;
+  exploded_page_state.top.http_body.http_content_type = u"text/plain";
+  std::string encoded_page_state;
+  blink::EncodePageState(exploded_page_state, &encoded_page_state);
+  blink::PageState page_state =
+      blink::PageState::CreateFromEncodedData(encoded_page_state);
+
+  // Create an interceptor which will put the modified PageState into the next
+  // navigation's DidCommit message.
+  DidCommitPageStateReplacer page_state_replacer(shell()->web_contents(),
+                                                 page_state);
+
+  // Navigate in the same renderer process to send the bad PageState.
+  RenderFrameHostImpl* rfh = static_cast<RenderFrameHostImpl*>(
+      shell()->web_contents()->GetPrimaryMainFrame());
+  RenderProcessHostBadIpcMessageWaiter kill_waiter(rfh->GetProcess());
+  EXPECT_TRUE(NavigateToURLAndExpectNoCommit(shell(), foo_url2));
+
+  // Verify that the malicious renderer was killed, for the right reason.
+  EXPECT_EQ(bad_message::RFH_CAN_ACCESS_FILES_OF_PAGE_STATE_AT_COMMIT,
+            kill_waiter.Wait());
+}
+
+// Similar to the test above, but also uses a malformed DocumentState within the
+// corrupted PageState, to make it harder to find file paths that are present.
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
+                       PageStateWithUnlistedFileAndBadDocumentState) {
+  // Navigate to foo.com initially.
+  GURL foo_url(embedded_test_server()->GetURL("foo.com", "/title1.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), foo_url));
+
+  // Create a PageState that contains a file path which isn't in the list of
+  // referenced files which are validated.
+  GURL foo_url2(embedded_test_server()->GetURL("foo.com", "/title2.html"));
+  blink::ExplodedPageState exploded_page_state;
+  ASSERT_TRUE(blink::DecodePageState(
+      blink::PageState::CreateFromURL(foo_url2).ToEncodedData(),
+      &exploded_page_state));
+  scoped_refptr<network::ResourceRequestBody> request_body =
+      new network::ResourceRequestBody();
+  base::FilePath bad_file = base::FilePath::FromUTF8Unsafe("/tmp/offlimits");
+  request_body->AppendFileRange(
+      bad_file, 0, std::numeric_limits<uint64_t>::max(), base::Time());
+  exploded_page_state.top.http_body.request_body = request_body;
+  exploded_page_state.top.http_body.http_content_type = u"text/plain";
+
+  // Also modify the DocumentState to force RecursivelyAppendReferencedFiles to
+  // return false.
+  exploded_page_state.top.document_state = {u"one", u"two"};
+
+  std::string encoded_page_state;
+  blink::EncodePageState(exploded_page_state, &encoded_page_state);
+  blink::PageState page_state =
+      blink::PageState::CreateFromEncodedData(encoded_page_state);
+
+  // Create an interceptor which will put the modified PageState into the next
+  // navigation's DidCommit message.
+  DidCommitPageStateReplacer page_state_replacer(shell()->web_contents(),
+                                                 page_state);
+
+  // Navigate in the same renderer process to send the bad PageState.

diff --git a/content/browser/security_exploit_browsertest.cc b/content/browser/security_exploit_browsertest.cc
index a0d347f..4d0c4617 100644
--- a/content/browser/security_exploit_browsertest.cc
+++ b/content/browser/security_exploit_browsertest.cc
@@ -90,6 +90,7 @@
 #include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
 #include "services/network/public/cpp/network_switches.h"
 #include "services/network/public/cpp/resource_request.h"
+#include "services/network/public/cpp/resource_request_body.h"
 #include "services/network/public/mojom/fetch_api.mojom.h"
 #include "services/network/public/mojom/trust_tokens.mojom.h"
 #include "services/network/public/mojom/url_loader.mojom.h"
@@ -102,6 +103,7 @@
 #include "third_party/blink/public/common/fenced_frame/fenced_frame_utils.h"
 #include "third_party/blink/public/common/frame/fenced_frame_sandbox_flags.h"
 #include "third_party/blink/public/common/navigation/navigation_policy.h"
+#include "third_party/blink/public/common/page_state/page_state_serialization.h"
 #include "third_party/blink/public/mojom/blob/blob_url_store.mojom.h"
 #include "third_party/blink/public/mojom/choosers/file_chooser.mojom.h"
 #include "third_party/blink/public/mojom/fenced_frame/fenced_frame.mojom.h"
@@ -1596,6 +1598,130 @@
   EXPECT_EQ(bad_message::RFH_INVALID_WEB_UI_CONTROLLER, kill_waiter.Wait());
 }
 
+namespace {
+
+// An interceptor class that allows replacing the PageState of the DidCommit IPC
+// from the renderer process to the browser process.
+class DidCommitPageStateReplacer : public DidCommitNavigationInterceptor {
+ public:
+  DidCommitPageStateReplacer(WebContents* web_contents,
+                             const blink::PageState& page_state)
+      : DidCommitNavigationInterceptor(web_contents),
+        replacement_page_state_(page_state) {}
+
+  DidCommitPageStateReplacer(const DidCommitPageStateReplacer&) = delete;
+  DidCommitPageStateReplacer& operator=(const DidCommitPageStateReplacer&) =
+      delete;
+
+  ~DidCommitPageStateReplacer() override = default;
+
+ protected:
+  bool WillProcessDidCommitNavigation(
+      RenderFrameHost* render_frame_host,
+      NavigationRequest* navigation_request,
+      mojom::DidCommitProvisionalLoadParamsPtr* params,
+      mojom::DidCommitProvisionalLoadInterfaceParamsPtr* interface_params)
+      override {
+    (**params).page_state = replacement_page_state_;
+    return true;
+  }
+
+ private:
+  blink::PageState replacement_page_state_;
+};
+
+}  // namespace
+
+// Test that committing a navigation with a PageState that does not list all of
+// its file paths in GetReferencedFiles will cause a renderer kill.
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest, PageStateWithUnlistedFile) {
+  // Navigate to foo.com initially.
+  GURL foo_url(embedded_test_server()->GetURL("foo.com", "/title1.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), foo_url));
+
+  // Create a PageState that contains a file path which isn't in the list of
+  // referenced files which are validated.
+  GURL foo_url2(embedded_test_server()->GetURL("foo.com", "/title2.html"));
+  blink::ExplodedPageState exploded_page_state;
+  ASSERT_TRUE(blink::DecodePageState(
+      blink::PageState::CreateFromURL(foo_url2).ToEncodedData(),
+      &exploded_page_state));
+  scoped_refptr<network::ResourceRequestBody> request_body =
+      new network::ResourceRequestBody();
+  base::FilePath bad_file = base::FilePath::FromUTF8Unsafe("/tmp/offlimits");
+  request_body->AppendFileRange(
+      bad_file, 0, std::numeric_limits<uint64_t>::max(), base::Time());
+  exploded_page_state.top.http_body.request_body = request_body;
+  exploded_page_state.top.http_body.http_content_type = u"text/plain";
+  std::string encoded_page_state;
+  blink::EncodePageState(exploded_page_state, &encoded_page_state);
+  blink::PageState page_state =
+      blink::PageState::CreateFromEncodedData(encoded_page_state);
+
+  // Create an interceptor which will put the modified PageState into the next
+  // navigation's DidCommit message.
+  DidCommitPageStateReplacer page_state_replacer(shell()->web_contents(),
+                                                 page_state);
+
+  // Navigate in the same renderer process to send the bad PageState.
+  RenderFrameHostImpl* rfh = static_cast<RenderFrameHostImpl*>(
+      shell()->web_contents()->GetPrimaryMainFrame());
+  RenderProcessHostBadIpcMessageWaiter kill_waiter(rfh->GetProcess());
+  EXPECT_TRUE(NavigateToURLAndExpectNoCommit(shell(), foo_url2));
+
+  // Verify that the malicious renderer was killed, for the right reason.
+  EXPECT_EQ(bad_message::RFH_CAN_ACCESS_FILES_OF_PAGE_STATE_AT_COMMIT,
+            kill_waiter.Wait());
+}
+
+// Similar to the test above, but also uses a malformed DocumentState within the
+// corrupted PageState, to make it harder to find file paths that are present.
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
+                       PageStateWithUnlistedFileAndBadDocumentState) {
+  // Navigate to foo.com initially.
+  GURL foo_url(embedded_test_server()->GetURL("foo.com", "/title1.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), foo_url));
+
+  // Create a PageState that contains a file path which isn't in the list of
+  // referenced files which are validated.
+  GURL foo_url2(embedded_test_server()->GetURL("foo.com", "/title2.html"));
+  blink::ExplodedPageState exploded_page_state;
+  ASSERT_TRUE(blink::DecodePageState(
+      blink::PageState::CreateFromURL(foo_url2).ToEncodedData(),
+      &exploded_page_state));
+  scoped_refptr<network::ResourceRequestBody> request_body =
+      new network::ResourceRequestBody();
+  base::FilePath bad_file = base::FilePath::FromUTF8Unsafe("/tmp/offlimits");
+  request_body->AppendFileRange(
+      bad_file, 0, std::numeric_limits<uint64_t>::max(), base::Time());
+  exploded_page_state.top.http_body.request_body = request_body;
+  exploded_page_state.top.http_body.http_content_type = u"text/plain";
+
+  // Also modify the DocumentState to force RecursivelyAppendReferencedFiles to
+  // return false.
+  exploded_page_state.top.document_state = {u"one", u"two"};
+
+  std::string encoded_page_state;
+  blink::EncodePageState(exploded_page_state, &encoded_page_state);
+  blink::PageState page_state =
+      blink::PageState::CreateFromEncodedData(encoded_page_state);
+
+  // Create an interceptor which will put the modified PageState into the next
+  // navigation's DidCommit message.
+  DidCommitPageStateReplacer page_state_replacer(shell()->web_contents(),
+                                                 page_state);
+
+  // Navigate in the same renderer process to send the bad PageState.
+  RenderFrameHostImpl* rfh = static_cast<RenderFrameHostImpl*>(
+      shell()->web_contents()->GetPrimaryMainFrame());
+  RenderProcessHostBadIpcMessageWaiter kill_waiter(rfh->GetProcess());
+  EXPECT_TRUE(NavigateToURLAndExpectNoCommit(shell(), foo_url2));
+
+  // Verify that the malicious renderer was killed, for the right reason.
+  EXPECT_EQ(bad_message::RFH_CAN_ACCESS_FILES_OF_PAGE_STATE_AT_COMMIT,
+            kill_waiter.Wait());
+}
+
 class BeginNavigationTransitionReplacer : public FrameHostInterceptor {
  public:
   BeginNavigationTransitionReplacer(WebContents* web_contents,