From 7a0ece0c748a4065084d64d991e25b447d5522ce Mon Sep 17 00:00:00 2001
From: Alex Moshchuk <[email protected]>
Date: Wed, 16 Sep 2026 22:27:54 -0700
Subject: [PATCH] Don't install FileSystemURLLoaderFactory for PDF documents

A PDF document commits a real origin, but its renderer process is
deliberately denied access to that origin's stored data
(ChildProcessSecurityPolicyImpl::IsAccessAllowedForPdfProcess() returns
false for kCanAccessDataForCommittedOrigin). As part of that, a PDF
document shouldn't be able to access filesystem: URLs.

This CL is a followup to https://crrev.com/c/8411305, which made
FileSystemEntryURLLoader::Start() check CanAccessDataForOrigin() instead
of CanCommitURL(), so a PDF document's filesystem: request is rejected
with net::ERR_INVALID_URL. It provides further hardening such that
RenderFrameHostImpl::CommitNavigation() no longer installs a
FileSystemURLLoaderFactory for PDF documents in the first place. Two
reasons:

- Defense in depth.  The guarantee no longer rests solely on the
  loader-side CPSP check.  A PDF document simply has no filesystem:
  factory to use.

- Cleaner failure mode.  Rather than reaching a browser-side loader
  that then rejects it, the request fails in the renderer as an
  unsupported scheme -- the same treatment content:// already receives
  a few lines above on Android (r1270300).

This is not an observable behavior change: from script, the request
fails either way. Accordingly, the added browser test
(FileSystemURLSubresourceBrowserTest.SubresourceLoadBlockedInPdfDocument)
passes with or without the CommitNavigation() change, and is added as
end-to-end regression coverage for the overall guarantee, and as the
only test that exercises the factory bundle documents actually receive
at commit time, rather than a factory constructed by the test.

Fixed: 549529981
Change-Id: I1366a3f733934ea9314cb3e35996cd7352abcb1f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8391596
Reviewed-by: Rakina Zata Amni <[email protected]>
Commit-Queue: Alex Moshchuk <[email protected]>
Reviewed-by: Eriko Kurimoto <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1699529}
---
diff --git a/content/browser/renderer_host/render_frame_host_impl.cc b/content/browser/renderer_host/render_frame_host_impl.cc
index 61feedc..6e495d4 100644
--- a/content/browser/renderer_host/render_frame_host_impl.cc
+++ b/content/browser/renderer_host/render_frame_host_impl.cc
@@ -13326,13 +13326,18 @@
     }
 #endif
 
-    auto* partition = GetStoragePartition();
-    non_network_factories.emplace(
-        url::kFileSystemScheme,
-        CreateFileSystemURLLoaderFactory(
-            GetProcess()->GetDeprecatedID(), GetFrameTreeNodeId(),
-            partition->GetFileSystemContext(), partition->GetPartitionDomain(),
-            commit_params->storage_key));
+    // Only non-PDF documents can load filesystem: subresources.  PDF
+    // documents may commit an origin, but their processes are not allowed to
+    // access stored data for it.
+    if (!navigation_request->GetUrlInfo().embedder_isolation_info.is_pdf()) {
+      auto* partition = GetStoragePartition();
+      non_network_factories.emplace(
+          url::kFileSystemScheme,
+          CreateFileSystemURLLoaderFactory(
+              GetProcess()->GetID().GetUnsafeValue(), GetFrameTreeNodeId(),
+              partition->GetFileSystemContext(),
+              partition->GetPartitionDomain(), commit_params->storage_key));
+    }
 
     non_network_factories.emplace(url::kDataScheme,
                                   DataURLLoaderFactory::Create());
diff --git a/content/browser/file_system/file_system_url_loader_factory_browsertest.cc b/content/browser/file_system/file_system_url_loader_factory_browsertest.cc
index 91dcf2b..4477056 100644
--- a/content/browser/file_system/file_system_url_loader_factory_browsertest.cc
+++ b/content/browser/file_system/file_system_url_loader_factory_browsertest.cc
@@ -36,15 +36,18 @@
 #include "components/file_access/scoped_file_access.h"
 #include "components/file_access/test/mock_scoped_file_access_delegate.h"
 #include "content/browser/process_lock.h"
+#include "content/browser/renderer_host/render_frame_host_impl.h"
 #include "content/browser/security/cpsp/child_process_security_policy_impl.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/browser/url_info.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/navigation_controller.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/content_browser_test.h"
+#include "content/public/test/content_browser_test_utils.h"
 #include "content/public/test/test_utils.h"
 #include "content/shell/browser/shell.h"
 #include "mojo/public/cpp/bindings/remote.h"
@@ -181,6 +184,20 @@
   return line.find("<script>start(\"") == 0;
 }
 
+// Commits `url` in `web_contents` as PDF content, so that the resulting
+// document runs in a process whose SiteInfo has `is_pdf` set.
+RenderFrameHostImpl* NavigateToURLAsPdf(WebContents* web_contents,
+                                        const GURL& url) {
+  NavigationController::LoadURLParams params(url);
+  params.transition_type = ui::PageTransitionFromInt(
+      ui::PAGE_TRANSITION_TYPED | ui::PAGE_TRANSITION_FROM_ADDRESS_BAR);
+  params.is_pdf = true;
+  NavigateToURLBlockUntilNavigationsComplete(
+      web_contents, params, 1,
+      /*ignore_uncommitted_navigations=*/false);
+  return static_cast<RenderFrameHostImpl*>(web_contents->GetPrimaryMainFrame());
+}
+
 }  // namespace
 
 class FileSystemURLLoaderFactoryTest
@@ -1099,4 +1116,63 @@
           kValidExternalMountPoint));
 }
 
+// Exercises filesystem: subresource loads through the loader factory that
+// documents actually receive at commit time, rather than through a factory
+// created directly by the test.
+class FileSystemURLSubresourceBrowserTest : public ContentBrowserTest {
+ protected:
+  void SetUpOnMainThread() override {
+    ContentBrowserTest::SetUpOnMainThread();
+    ASSERT_TRUE(embedded_test_server()->Start());
+  }
+};
+
+// Verify that a document committed as PDF content cannot fetch filesystem:
+// subresources for its origin, while an ordinary document at the same origin
+// can.
+IN_PROC_BROWSER_TEST_F(FileSystemURLSubresourceBrowserTest,
+                       SubresourceLoadBlockedInPdfDocument) {
+  const GURL page_url = embedded_test_server()->GetURL("/title1.html");
+  EXPECT_TRUE(NavigateToURL(shell(), page_url));
+
+  // Create a temporary file from an ordinary document.
+  static constexpr char kWriteScript[] = R"(
+      new Promise((resolve, reject) => {
+        webkitRequestFileSystem(TEMPORARY, 1024, fs => {
+          fs.root.getFile('subresource.txt', {create: true}, entry => {
+            entry.createWriter(writer => {
+              writer.onwriteend = () => resolve('written');
+              writer.onerror = () => reject(writer.error);
+              writer.write(new Blob(['fs-data']));
+            }, reject);
+          }, reject);
+        }, reject);
+      });)";
+  ASSERT_EQ("written", EvalJs(shell(), kWriteScript));
+
+  static constexpr char kReadScript[] = R"(
+      new Promise(resolve => {
+        const request = new XMLHttpRequest();
+        request.open('GET', 'filesystem:' + location.origin +
+                                '/temporary/subresource.txt');
+        request.onload = () => resolve(request.responseText);
+        request.onerror = () => resolve('load-error');
+        request.send();
+      });)";
+
+  // The ordinary document can read the file back through its filesystem: URL.
+  ASSERT_EQ("fs-data", EvalJs(shell(), kReadScript));
+
+  // Commit a same-origin document as PDF content, so that it runs in a
+  // process whose SiteInfo has `is_pdf` set.
+  const GURL pdf_url = embedded_test_server()->GetURL("/title2.html");
+  RenderFrameHostImpl* frame =
+      NavigateToURLAsPdf(shell()->web_contents(), pdf_url);
+  ASSERT_EQ(pdf_url, shell()->web_contents()->GetLastCommittedURL());
+  ASSERT_TRUE(frame->GetSiteInstance()->GetSiteInfo().is_pdf());
+
+  // The PDF document must not receive the file's contents.
+  EXPECT_EQ("load-error", EvalJs(frame, kReadScript));
+}
+
 }  // namespace content

diff --git a/content/browser/file_system/file_system_url_loader_factory_browsertest.cc b/content/browser/file_system/file_system_url_loader_factory_browsertest.cc
index 91dcf2b..4477056 100644
--- a/content/browser/file_system/file_system_url_loader_factory_browsertest.cc
+++ b/content/browser/file_system/file_system_url_loader_factory_browsertest.cc
@@ -36,15 +36,18 @@
 #include "components/file_access/scoped_file_access.h"
 #include "components/file_access/test/mock_scoped_file_access_delegate.h"
 #include "content/browser/process_lock.h"
+#include "content/browser/renderer_host/render_frame_host_impl.h"
 #include "content/browser/security/cpsp/child_process_security_policy_impl.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/browser/url_info.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/navigation_controller.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/content_browser_test.h"
+#include "content/public/test/content_browser_test_utils.h"
 #include "content/public/test/test_utils.h"
 #include "content/shell/browser/shell.h"
 #include "mojo/public/cpp/bindings/remote.h"
@@ -181,6 +184,20 @@
   return line.find("<script>start(\"") == 0;
 }
 
+// Commits `url` in `web_contents` as PDF content, so that the resulting
+// document runs in a process whose SiteInfo has `is_pdf` set.
+RenderFrameHostImpl* NavigateToURLAsPdf(WebContents* web_contents,
+                                        const GURL& url) {
+  NavigationController::LoadURLParams params(url);
+  params.transition_type = ui::PageTransitionFromInt(
+      ui::PAGE_TRANSITION_TYPED | ui::PAGE_TRANSITION_FROM_ADDRESS_BAR);
+  params.is_pdf = true;
+  NavigateToURLBlockUntilNavigationsComplete(
+      web_contents, params, 1,
+      /*ignore_uncommitted_navigations=*/false);
+  return static_cast<RenderFrameHostImpl*>(web_contents->GetPrimaryMainFrame());
+}
+
 }  // namespace
 
 class FileSystemURLLoaderFactoryTest
@@ -1099,4 +1116,63 @@
           kValidExternalMountPoint));
 }
 
+// Exercises filesystem: subresource loads through the loader factory that
+// documents actually receive at commit time, rather than through a factory
+// created directly by the test.
+class FileSystemURLSubresourceBrowserTest : public ContentBrowserTest {
+ protected:
+  void SetUpOnMainThread() override {
+    ContentBrowserTest::SetUpOnMainThread();
+    ASSERT_TRUE(embedded_test_server()->Start());
+  }
+};
+
+// Verify that a document committed as PDF content cannot fetch filesystem:
+// subresources for its origin, while an ordinary document at the same origin
+// can.
+IN_PROC_BROWSER_TEST_F(FileSystemURLSubresourceBrowserTest,
+                       SubresourceLoadBlockedInPdfDocument) {
+  const GURL page_url = embedded_test_server()->GetURL("/title1.html");
+  EXPECT_TRUE(NavigateToURL(shell(), page_url));
+
+  // Create a temporary file from an ordinary document.
+  static constexpr char kWriteScript[] = R"(
+      new Promise((resolve, reject) => {
+        webkitRequestFileSystem(TEMPORARY, 1024, fs => {
+          fs.root.getFile('subresource.txt', {create: true}, entry => {
+            entry.createWriter(writer => {
+              writer.onwriteend = () => resolve('written');
+              writer.onerror = () => reject(writer.error);
+              writer.write(new Blob(['fs-data']));
+            }, reject);
+          }, reject);
+        }, reject);
+      });)";
+  ASSERT_EQ("written", EvalJs(shell(), kWriteScript));
+
+  static constexpr char kReadScript[] = R"(
+      new Promise(resolve => {
+        const request = new XMLHttpRequest();
+        request.open('GET', 'filesystem:' + location.origin +
+                                '/temporary/subresource.txt');
+        request.onload = () => resolve(request.responseText);
+        request.onerror = () => resolve('load-error');
+        request.send();
+      });)";
+
+  // The ordinary document can read the file back through its filesystem: URL.
+  ASSERT_EQ("fs-data", EvalJs(shell(), kReadScript));
+
+  // Commit a same-origin document as PDF content, so that it runs in a
+  // process whose SiteInfo has `is_pdf` set.
+  const GURL pdf_url = embedded_test_server()->GetURL("/title2.html");
+  RenderFrameHostImpl* frame =
+      NavigateToURLAsPdf(shell()->web_contents(), pdf_url);
+  ASSERT_EQ(pdf_url, shell()->web_contents()->GetLastCommittedURL());
+  ASSERT_TRUE(frame->GetSiteInstance()->GetSiteInfo().is_pdf());
+
+  // The PDF document must not receive the file's contents.
+  EXPECT_EQ("load-error", EvalJs(frame, kReadScript));
+}
+
 }  // namespace content