PatchGap
Chrome · File System Access

CVE-2026-12460

Logic Error in File System Access
High chrome Logic Error 📄 Reporter bug report 🔧 Commit mapped

Overview

High
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactInsufficient policy enforcement in File System Access
DescriptionInsufficient policy enforcement in File System Access
ComponentFile System Access
Bug ClassLogic Error
Tracker517484284
Fix commit732be71ad03c (chromium/src) +136/-55
CISA KEVNot listed
CreditedGoogle
Disclosed2026-06-16

Files Changed

  • content/browser/file_system_access/file_system_access_file_writer_impl_unittest.cc
  • content/browser/file_system_access/file_system_access_manager_impl.cc
  • content/browser/file_system_access/file_system_access_manager_impl_unittest.cc

Patch

view commit
From 732be71ad03cefa1d8d6f0e3d04774991ecf9940 Mon Sep 17 00:00:00 2001
From: Fergal Daly <fergal@chromium.org>
Date: Wed, 03 Jun 2026 22:15:47 -0700
Subject: [PATCH] Block PDF processes from accessing OPFS

FileSystemAccessManagerImpl::GetSandboxedFileSystem did not check if the
calling process has permission to access the storage key's origin. Under
Site Isolation, PDF renderers are locked to the PDF-serving origin but
are isolated from accessing persistent storage, cookies, and other host
origin data.

This CL adds a ChildProcessSecurityPolicy check to
GetSandboxedFileSystem to prevent PDF-isolated processes from accessing
the Origin-Private File System (OPFS).

Update some unittests to use a more realistic BindingContext now that
the code refers to the binding context.

The new tests fails without the fix applied.

Fixed: 517484284

TAG=agy CONV=18603561-8bc0-4ac5-9257-7ba3b948a1ef

Change-Id: Ib7018f3d3693c861865d752fdc320bc832cfd4ad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7894258
Auto-Submit: Fergal Daly <fergal@chromium.org>
Commit-Queue: Ming-Ying Chung <mych@chromium.org>
Reviewed-by: Ming-Ying Chung <mych@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1641471}
---

diff --git a/content/browser/file_system_access/file_system_access_file_writer_impl_unittest.cc b/content/browser/file_system_access/file_system_access_file_writer_impl_unittest.cc
index 3d39a50..eeb8d0a 100644
--- a/content/browser/file_system_access/file_system_access_file_writer_impl_unittest.cc
+++ b/content/browser/file_system_access/file_system_access_file_writer_impl_unittest.cc
@@ -26,11 +26,13 @@
 #include "components/services/storage/public/cpp/buckets/bucket_info.h"
 #include "components/services/storage/public/cpp/buckets/bucket_locator.h"
 #include "content/browser/blob_storage/chrome_blob_storage_context.h"
+#include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/file_system_access/file_system_access_lock_manager.h"
 #include "content/browser/file_system_access/fixed_file_system_access_permission_grant.h"
 #include "content/browser/file_system_access/mock_file_system_access_permission_context.h"
 #include "content/browser/file_system_access/mock_file_system_access_permission_grant.h"
 #include "content/public/test/browser_task_environment.h"
+#include "content/public/test/test_browser_context.h"
 #include "mojo/public/cpp/system/data_pipe_producer.h"
 #include "mojo/public/cpp/system/string_data_source.h"
 #include "net/base/io_buffer.h"
@@ -132,6 +134,9 @@
   void TearDown() override {
     manager_.reset();
 
+    ChildProcessSecurityPolicyImpl::GetInstance()->Remove(
+        ChildProcessId::FromUnsafeValue(kProcessId));
+
     task_environment_.RunUntilIdle();
     // TODO(crbug.com/40266589): Figure out what code is leaking open
     // files, and uncomment this to prevent further regressions.
@@ -293,6 +298,10 @@
         file_system_context_, chrome_blob_context_,
         /*permission_context=*/permission_context(),
         /*off_the_record=*/false);
+    ChildProcessSecurityPolicyImpl::GetInstance()->AddForTesting(
+        ChildProcessId::FromUnsafeValue(kProcessId), &browser_context_);
+    ChildProcessSecurityPolicyImpl::GetInstance()->AddCommittedOrigin(
+        kProcessId, url::Origin::Create(kTestURL));
     manager_->BindReceiver(kBindingContext,
                            manager_remote_.BindNewPipeAndPassReceiver());
 
@@ -388,6 +397,7 @@
   raw_ptr<storage::BlobStorageContext> blob_context_ = nullptr;
   scoped_refptr<FileSystemAccessManagerImpl> manager_;
   mojo::Remote<blink::mojom::FileSystemAccessManager> manager_remote_;
+  TestBrowserContext browser_context_;
 
   FileSystemURL test_file_url_;
   FileSystemURL test_swap_url_;
diff --git a/content/browser/file_system_access/file_system_access_manager_impl.cc b/content/browser/file_system_access/file_system_access_manager_impl.cc
index 51049e7..52bccb1 100644
--- a/content/browser/file_system_access/file_system_access_manager_impl.cc
+++ b/content/browser/file_system_access/file_system_access_manager_impl.cc
@@ -49,6 +49,7 @@
 #include "content/browser/renderer_host/frame_tree_node.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/child_process_security_policy.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
@@ -571,6 +572,14 @@
     GetSandboxedFileSystemCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
+  if (!ChildProcessSecurityPolicy::GetInstance()->CanAccessDataForOrigin(
+          binding_context.process_id(), binding_context.storage_key.origin())) {
+    std::move(callback).Run(file_system_access_error::FromFileError(
+                                base::File::FILE_ERROR_SECURITY),
+                            mojo::NullRemote());
+    return;
+  }
+
   auto response_callback = base::BindOnce(
       [](base::WeakPtr<FileSystemAccessManagerImpl> manager,
          const BindingContext& callback_binding_context,
diff --git a/content/browser/file_system_access/file_system_access_manager_impl_unittest.cc b/content/browser/file_system_access/file_system_access_manager_impl_unittest.cc
index b6037546..cc846055 100644
--- a/content/browser/file_system_access/file_system_access_manager_impl_unittest.cc
+++ b/content/browser/file_system_access/file_system_access_manager_impl_unittest.cc
@@ -29,6 +29,7 @@
 #include "components/services/storage/public/cpp/buckets/bucket_locator.h"
 #include "components/services/storage/public/cpp/buckets/constants.h"
 #include "content/browser/blob_storage/chrome_blob_storage_context.h"
+#include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/file_system_access/features.h"
 #include "content/browser/file_system_access/file_system_access_data_transfer_token_impl.h"
 #include "content/browser/file_system_access/file_system_access_directory_handle_impl.h"
@@ -202,7 +203,10 @@
     web_contents_ = web_contents_factory_.CreateWebContents(&browser_context_);
     static_cast<TestWebContents*>(web_contents_)->NavigateAndCommit(kTestURL);
 
-    manager_->BindReceiver(kBindingContext,
+    frame_id_ = web_contents_->GetPrimaryMainFrame()->GetGlobalId();
+    binding_context_ = {kTestStorageKey, kTestURL, frame_id_};
+
+    manager_->BindReceiver(binding_context_,
                            manager_remote_.BindNewPipeAndPassReceiver());
 
     EXPECT_CALL(permission_context_, IsFileTypeDangerous_)
@@ -245,7 +249,7 @@
 
     blink::mojom::FileSystemAccessEntryPtr entry =
         manager_->CreateDirectoryEntryFromPath(
-            kBindingContext, path_info,
+            binding_context_, path_info,
             FileSystemAccessPermissionContext::UserAction::kOpen);
     return mojo::Remote<blink::mojom::FileSystemAccessDirectoryHandle>(
         std::move(entry->entry_handle->get_directory()));
@@ -276,7 +280,7 @@
     mojo::PendingRemote<blink::mojom::FileSystemAccessDataTransferToken>
         token_remote;
     manager_->CreateFileSystemAccessDataTransferToken(
-        file_path_info, kBindingContext.process_id(),
+        file_path_info, binding_context_.process_id(),
         token_remote.InitWithNewPipeAndPassReceiver());
 
     // Expect permission requests when the token is sent to be redeemed.
@@ -322,7 +326,7 @@
     mojo::PendingRemote<blink::mojom::FileSystemAccessDataTransferToken>
         token_remote;
     manager_->CreateFileSystemAccessDataTransferToken(
-        dir_path_info, kBindingContext.process_id(),
+        dir_path_info, binding_context_.process_id(),
         token_remote.InitWithNewPipeAndPassReceiver());
 
     EXPECT_CALL(permission_context_,
@@ -330,7 +334,7 @@
                     kTestStorageKey.origin(), dir_path_info,
                     FileSystemAccessPermissionContext::HandleType::kDirectory,
                     FileSystemAccessPermissionContext::UserAction::kDragAndDrop,
-                    kFrameId, testing::_))
+                    frame_id_, testing::_))
         .WillOnce(RunOnceCallback<5>(
             FileSystemAccessPermissionContext::SensitiveEntryResult::kAllowed));
 
@@ -432,12 +436,11 @@
  protected:
   const GURL kTestURL = GURL("https://example.com/test");
   const blink::StorageKey kTestStorageKey =
-      blink::StorageKey::CreateFromStringForTesting("https://example.com/test");
-  const int kProcessId = 1;
-  const int kFrameRoutingId = 2;
-  const GlobalRenderFrameHostId kFrameId{kProcessId, kFrameRoutingId};
-  const FileSystemAccessManagerImpl::BindingContext kBindingContext = {
-      kTestStorageKey, kTestURL, kFrameId};
+      blink::StorageKey::CreateFromStringForTesting(kTestURL.spec());
+  GlobalRenderFrameHostId frame_id_;
+  // An initial value is required but this is overwritten by `SetUp`.
+  FileSystemAccessManagerImpl::BindingContext binding_context_ = {
+      kTestStorageKey, kTestURL, GlobalRenderFrameHostId()};
 
   scoped_refptr<storage::MockSpecialStoragePolicy> special_storage_policy_;
 
@@ -632,7 +635,7 @@
 
   blink::mojom::FileSystemAccessEntryPtr entry =
       manager_->CreateFileEntryFromPath(
-          kBindingContext, kTestPathInfo,
+          binding_context_, kTestPathInfo,
           FileSystemAccessPermissionContext::UserAction::kOpen);
   mojo::Remote<blink::mojom::FileSystemAccessFileHandle> handle(
       std::move(entry->entry_handle->get_file()));
@@ -664,7 +667,7 @@
 
   blink::mojom::FileSystemAccessEntryPtr entry =
       manager_->CreateFileEntryFromPath(
-          kBindingContext, kTestPathInfo,
+          binding_context_, kTestPathInfo,
           FileSystemAccessPermissionContext::UserAction::kSave);
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/content/browser/file_system_access/file_system_access_file_writer_impl_unittest.cc b/content/browser/file_system_access/file_system_access_file_writer_impl_unittest.cc
index 3d39a50..eeb8d0a 100644
--- a/content/browser/file_system_access/file_system_access_file_writer_impl_unittest.cc
+++ b/content/browser/file_system_access/file_system_access_file_writer_impl_unittest.cc
@@ -26,11 +26,13 @@
 #include "components/services/storage/public/cpp/buckets/bucket_info.h"
 #include "components/services/storage/public/cpp/buckets/bucket_locator.h"
 #include "content/browser/blob_storage/chrome_blob_storage_context.h"
+#include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/file_system_access/file_system_access_lock_manager.h"
 #include "content/browser/file_system_access/fixed_file_system_access_permission_grant.h"
 #include "content/browser/file_system_access/mock_file_system_access_permission_context.h"
 #include "content/browser/file_system_access/mock_file_system_access_permission_grant.h"
 #include "content/public/test/browser_task_environment.h"
+#include "content/public/test/test_browser_context.h"
 #include "mojo/public/cpp/system/data_pipe_producer.h"
 #include "mojo/public/cpp/system/string_data_source.h"
 #include "net/base/io_buffer.h"
@@ -132,6 +134,9 @@
   void TearDown() override {
     manager_.reset();
 
+    ChildProcessSecurityPolicyImpl::GetInstance()->Remove(
+        ChildProcessId::FromUnsafeValue(kProcessId));
+
     task_environment_.RunUntilIdle();
     // TODO(crbug.com/40266589): Figure out what code is leaking open
     // files, and uncomment this to prevent further regressions.
@@ -293,6 +298,10 @@
         file_system_context_, chrome_blob_context_,
         /*permission_context=*/permission_context(),
         /*off_the_record=*/false);
+    ChildProcessSecurityPolicyImpl::GetInstance()->AddForTesting(
+        ChildProcessId::FromUnsafeValue(kProcessId), &browser_context_);
+    ChildProcessSecurityPolicyImpl::GetInstance()->AddCommittedOrigin(
+        kProcessId, url::Origin::Create(kTestURL));
     manager_->BindReceiver(kBindingContext,
                            manager_remote_.BindNewPipeAndPassReceiver());
 
@@ -388,6 +397,7 @@
   raw_ptr<storage::BlobStorageContext> blob_context_ = nullptr;
   scoped_refptr<FileSystemAccessManagerImpl> manager_;
   mojo::Remote<blink::mojom::FileSystemAccessManager> manager_remote_;
+  TestBrowserContext browser_context_;
 
   FileSystemURL test_file_url_;
   FileSystemURL test_swap_url_;
diff --git a/content/browser/file_system_access/file_system_access_manager_impl_unittest.cc b/content/browser/file_system_access/file_system_access_manager_impl_unittest.cc
index b6037546..cc846055 100644
--- a/content/browser/file_system_access/file_system_access_manager_impl_unittest.cc
+++ b/content/browser/file_system_access/file_system_access_manager_impl_unittest.cc
@@ -29,6 +29,7 @@
 #include "components/services/storage/public/cpp/buckets/bucket_locator.h"
 #include "components/services/storage/public/cpp/buckets/constants.h"
 #include "content/browser/blob_storage/chrome_blob_storage_context.h"
+#include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/file_system_access/features.h"
 #include "content/browser/file_system_access/file_system_access_data_transfer_token_impl.h"
 #include "content/browser/file_system_access/file_system_access_directory_handle_impl.h"
@@ -202,7 +203,10 @@
     web_contents_ = web_contents_factory_.CreateWebContents(&browser_context_);
     static_cast<TestWebContents*>(web_contents_)->NavigateAndCommit(kTestURL);
 
-    manager_->BindReceiver(kBindingContext,
+    frame_id_ = web_contents_->GetPrimaryMainFrame()->GetGlobalId();
+    binding_context_ = {kTestStorageKey, kTestURL, frame_id_};
+
+    manager_->BindReceiver(binding_context_,
                            manager_remote_.BindNewPipeAndPassReceiver());
 
     EXPECT_CALL(permission_context_, IsFileTypeDangerous_)
@@ -245,7 +249,7 @@
 
     blink::mojom::FileSystemAccessEntryPtr entry =
         manager_->CreateDirectoryEntryFromPath(
-            kBindingContext, path_info,
+            binding_context_, path_info,
             FileSystemAccessPermissionContext::UserAction::kOpen);
     return mojo::Remote<blink::mojom::FileSystemAccessDirectoryHandle>(
         std::move(entry->entry_handle->get_directory()));
@@ -276,7 +280,7 @@
     mojo::PendingRemote<blink::mojom::FileSystemAccessDataTransferToken>
         token_remote;
     manager_->CreateFileSystemAccessDataTransferToken(
-        file_path_info, kBindingContext.process_id(),
+        file_path_info, binding_context_.process_id(),
         token_remote.InitWithNewPipeAndPassReceiver());
 
     // Expect permission requests when the token is sent to be redeemed.
@@ -322,7 +326,7 @@
     mojo::PendingRemote<blink::mojom::FileSystemAccessDataTransferToken>
         token_remote;
     manager_->CreateFileSystemAccessDataTransferToken(
-        dir_path_info, kBindingContext.process_id(),
+        dir_path_info, binding_context_.process_id(),
         token_remote.InitWithNewPipeAndPassReceiver());
 
     EXPECT_CALL(permission_context_,
@@ -330,7 +334,7 @@
                     kTestStorageKey.origin(), dir_path_info,
                     FileSystemAccessPermissionContext::HandleType::kDirectory,
                     FileSystemAccessPermissionContext::UserAction::kDragAndDrop,
-                    kFrameId, testing::_))
+                    frame_id_, testing::_))
         .WillOnce(RunOnceCallback<5>(
             FileSystemAccessPermissionContext::SensitiveEntryResult::kAllowed));
 
@@ -432,12 +436,11 @@
  protected:
   const GURL kTestURL = GURL("https://example.com/test");
   const blink::StorageKey kTestStorageKey =
-      blink::StorageKey::CreateFromStringForTesting("https://example.com/test");
-  const int kProcessId = 1;
-  const int kFrameRoutingId = 2;
-  const GlobalRenderFrameHostId kFrameId{kProcessId, kFrameRoutingId};
-  const FileSystemAccessManagerImpl::BindingContext kBindingContext = {
-      kTestStorageKey, kTestURL, kFrameId};
+      blink::StorageKey::CreateFromStringForTesting(kTestURL.spec());
+  GlobalRenderFrameHostId frame_id_;
+  // An initial value is required but this is overwritten by `SetUp`.
+  FileSystemAccessManagerImpl::BindingContext binding_context_ = {
+      kTestStorageKey, kTestURL, GlobalRenderFrameHostId()};
 
   scoped_refptr<storage::MockSpecialStoragePolicy> special_storage_policy_;
 
@@ -632,7 +635,7 @@
 
   blink::mojom::FileSystemAccessEntryPtr entry =
       manager_->CreateFileEntryFromPath(
-          kBindingContext, kTestPathInfo,
+          binding_context_, kTestPathInfo,
           FileSystemAccessPermissionContext::UserAction::kOpen);
   mojo::Remote<blink::mojom::FileSystemAccessFileHandle> handle(
       std::move(entry->entry_handle->get_file()));
@@ -664,7 +667,7 @@
 
   blink::mojom::FileSystemAccessEntryPtr entry =
       manager_->CreateFileEntryFromPath(
-          kBindingContext, kTestPathInfo,
+          binding_context_, kTestPathInfo,
           FileSystemAccessPermissionContext::UserAction::kSave);
   mojo::Remote<blink::mojom::FileSystemAccessFileHandle> handle(
       std::move(entry->entry_handle->get_file()));
@@ -698,7 +701,7 @@
 
   blink::mojom::FileSystemAccessEntryPtr entry =
       manager_->CreateDirectoryEntryFromPath(
-          kBindingContext, kTestPathInfo,
+          binding_context_, kTestPathInfo,
           FileSystemAccessPermissionContext::UserAction::kOpen);
   mojo::Remote<blink::mojom::FileSystemAccessDirectoryHandle> handle(
       std::move(entry->entry_handle->get_directory()));
@@ -730,15 +733,15 @@
             storage::AsyncFileTestHelper::CreateFile(file_system_context_.get(),
                                                      test_swap_url));
 
-  auto lock = TakeLockSync(kBindingContext, test_file_url,
+  auto lock = TakeLockSync(binding_context_, test_file_url,
                            manager_->GetWFSSiloedLockType());
   ASSERT_TRUE(lock);
-  auto swap_lock = TakeLockSync(kBindingContext, test_swap_url,
+  auto swap_lock = TakeLockSync(binding_context_, test_swap_url,
                                 manager_->GetExclusiveLockType());
   ASSERT_TRUE(swap_lock);
 
   mojo::Remote<blink::mojom::FileSystemAccessFileWriter> writer_remote(
-      manager_->CreateFileWriter(kBindingContext, test_file_url, test_swap_url,
+      manager_->CreateFileWriter(binding_context_, test_file_url, test_swap_url,
                                  std::move(lock), std::move(swap_lock),
                                  FileSystemAccessManagerImpl::SharedHandleState(
                                      allow_grant_, allow_grant_),
@@ -771,15 +774,15 @@
             storage::AsyncFileTestHelper::CreateFileWithData(
                 file_system_context_.get(), test_swap_url, "foo"));
 
-  auto lock = TakeLockSync(kBindingContext, test_file_url,
+  auto lock = TakeLockSync(binding_context_, test_file_url,
                            manager_->GetWFSSiloedLockType());
   ASSERT_TRUE(lock);
-  auto swap_lock = TakeLockSync(kBindingContext, test_swap_url,
+  auto swap_lock = TakeLockSync(binding_context_, test_swap_url,
                                 manager_->GetExclusiveLockType());
   ASSERT_TRUE(swap_lock);
 
   mojo::Remote<blink::mojom::FileSystemAccessFileWriter> writer_remote(
-      manager_->CreateFileWriter(kBindingContext, test_file_url, test_swap_url,
+      manager_->CreateFileWriter(binding_context_, test_file_url, test_swap_url,
                                  std::move(lock), std::move(swap_lock),
                                  FileSystemAccessManagerImpl::SharedHandleState(
                                      allow_grant_, allow_grant_),
@@ -792,7 +795,7 @@
   writer_remote->Close(base::DoNothing());
 
   EXPECT_CALL(permission_context_,
-              PerformAfterWriteChecks_(testing::_, kFrameId, testing::_))
+              PerformAfterWriteChecks_(testing::_, frame_id_, testing::_))
       .WillOnce(base::test::RunOnceCallback<2>(
           FileSystemAccessPermissionContext::AfterWriteCheckResult::kAllow));
 
@@ -823,15 +826,15 @@
             storage::AsyncFileTestHelper::CreateFileWithData(
                 file_system_context_.get(), test_swap_url, "foo"));
 
-  auto lock = TakeLockSync(kBindingContext, test_file_url,
+  auto lock = TakeLockSync(binding_context_, test_file_url,
                            manager_->GetWFSSiloedLockType());
   ASSERT_TRUE(lock);
-  auto swap_lock = TakeLockSync(kBindingContext, test_swap_url,
+  auto swap_lock = TakeLockSync(binding_context_, test_swap_url,
                                 manager_->GetExclusiveLockType());
   ASSERT_TRUE(swap_lock);
 
   mojo::Remote<blink::mojom::FileSystemAccessFileWriter> writer_remote(
-      manager_->CreateFileWriter(kBindingContext, test_file_url, test_swap_url,
+      manager_->CreateFileWriter(binding_context_, test_file_url, test_swap_url,
                                  std::move(lock), std::move(swap_lock),
                                  FileSystemAccessManagerImpl::SharedHandleState(
                                      allow_grant_, allow_grant_),
@@ -864,15 +867,15 @@
             storage::AsyncFileTestHelper::CreateFileWithData(
                 file_system_context_.get(), test_swap_url, "foo"));
 
-  auto lock = TakeLockSync(kBindingContext, test_file_url,
+  auto lock = TakeLockSync(binding_context_, test_file_url,
                            manager_->GetWFSSiloedLockType());
   ASSERT_TRUE(lock);
-  auto swap_lock = TakeLockSync(kBindingContext, test_swap_url,
+  auto swap_lock = TakeLockSync(binding_context_, test_swap_url,
                                 manager_->GetExclusiveLockType());
   ASSERT_TRUE(swap_lock);
 
   mojo::Remote<blink::mojom::FileSystemAccessFileWriter> writer_remote(
-      manager_->CreateFileWriter(kBindingContext, test_file_url, test_swap_url,
+      manager_->CreateFileWriter(binding_context_, test_file_url, test_swap_url,
                                  std::move(lock), std::move(swap_lock),
                                  FileSystemAccessManagerImpl::SharedHandleState(
                                      allow_grant_, allow_grant_),
@@ -884,7 +887,7 @@
       storage::AsyncFileTestHelper::kDontCheckSize));
 
   EXPECT_CALL(permission_context_,
-              PerformAfterWriteChecks_(testing::_, kFrameId, testing::_))
+              PerformAfterWriteChecks_(testing::_, frame_id_, testing::_))
       .WillOnce(base::test::RunOnceCallback<2>(
           FileSystemAccessPermissionContext::AfterWriteCheckResult::kAllow));
 
@@ -910,7 +913,7 @@
       kTestStorageKey, storage::kFileSystemTypeTemporary,
       base::FilePath::FromUTF8Unsafe("test/foo/bar"));
   test_file_url.SetBucket(default_bucket);
-  FileSystemAccessFileHandleImpl file(manager_.get(), kBindingContext,
+  FileSystemAccessFileHandleImpl file(manager_.get(), binding_context_,
                                       test_file_url, "bar",
                                       {ask_grant_, ask_grant_});
   mojo::PendingRemote<blink::mojom::FileSystemAccessTransferToken> token_remote;
@@ -939,7 +942,7 @@
       base::FilePath::FromUTF8Unsafe("test/foo/bar"));
   ASSERT_OK_AND_ASSIGN(auto bucket, CreateBucketForTesting());
   test_file_url.SetBucket(std::move(bucket));
-  FileSystemAccessFileHandleImpl file(manager_.get(), kBindingContext,
+  FileSystemAccessFileHandleImpl file(manager_.get(), binding_context_,
                                       test_file_url, "bar",
                                       {ask_grant_, ask_grant_});
   mojo::PendingRemote<blink::mojom::FileSystemAccessTransferToken> token_remote;
@@ -969,8 +972,9 @@
       kTestStorageKey, storage::kFileSystemTypeTemporary,
       base::FilePath::FromUTF8Unsafe("hello/world/"));
   test_file_url.SetBucket(default_bucket);
-  FileSystemAccessDirectoryHandleImpl directory(
-      manager_.get(), kBindingContext, test_file_url, {ask_grant_, ask_grant_});
+  FileSystemAccessDirectoryHandleImpl directory(manager_.get(),
+                                                binding_context_, test_file_url,
+                                                {ask_grant_, ask_grant_});
   mojo::PendingRemote<blink::mojom::FileSystemAccessTransferToken> token_remote;
   manager_->CreateTransferToken(directory,
                                 token_remote.InitWithNewPipeAndPassReceiver());
@@ -997,8 +1001,9 @@
       base::FilePath::FromUTF8Unsafe("hello/world/"));
   ASSERT_OK_AND_ASSIGN(auto bucket, CreateBucketForTesting());
   test_file_url.SetBucket(std::move(bucket));
-  FileSystemAccessDirectoryHandleImpl directory(
-      manager_.get(), kBindingContext, test_file_url, {ask_grant_, ask_grant_});
+  FileSystemAccessDirectoryHandleImpl directory(manager_.get(),
+                                                binding_context_, test_file_url,
+                                                {ask_grant_, ask_grant_});
   mojo::PendingRemote<blink::mojom::FileSystemAccessTransferToken> token_remote;
   manager_->CreateTransferToken(directory,
                                 token_remote.InitWithNewPipeAndPassReceiver());
@@ -1039,7 +1044,7 @@
 
   blink::mojom::FileSystemAccessEntryPtr entry =
       manager_->CreateFileEntryFromPath(
-          kBindingContext, kTestPathInfo,
+          binding_context_, kTestPathInfo,
           FileSystemAccessPermissionContext::UserAction::kOpen);
   mojo::Remote<blink::mojom::FileSystemAccessFileHandle> handle(
       std::move(entry->entry_handle->get_file()));
@@ -1123,7 +1128,7 @@
                     PathInfo(kDirectoryPathInfo.path.AppendASCII(kTestName)),
                     FileSystemAccessPermissionContext::HandleType::kFile,
... (truncated)
Loading diff…

Original Bug Report

reported by vm...@google.com

Bypass of PDF storage isolation restrictions via FileSystemAccessManager

Project Fortify, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports. Please see https://chromium.googlesource.com/chromium/src/+/main/docs/security/ai-generated-security-bugs-faq.md for more information.

Overview: A potential vulnerability exists where a compromised PDF renderer process can bypass storage restrictions and access the serving origin’s Origin-Private File System (OPFS). This is caused by a missing ChildProcessSecurityPolicy check inside FileSystemAccessManagerImpl::GetSandboxedFileSystem. A compromised PDF process could exploit this to read, write, or delete files in the origin’s OPFS.

Affected files:

  • content/browser/file_system_access/file_system_access_manager_impl.cc

Estimated timestamp from git blame: 2024-06-26

Description

Under Chromium’s Site Isolation security architecture, PDF renderers (run in processes with ProcessLock.is_pdf() == true) are isolated from accessing persistent storage, cookies, and other host origin data. This security boundary is designed to limit the impact of a potential compromise of the highly exposed PDF engine.

While the legacy FileSystem API (FileSystemManagerImpl::Open) correctly enforces this isolation by verifying access through ChildProcessSecurityPolicyImpl::Handle::CanAccessDataForOrigin, the modern FileSystemAccess API (FileSystemAccessManagerImpl::GetSandboxedFileSystem) does not perform any equivalent validation. As a result, a compromised PDF renderer can potentially obtain a handle to the serving origin’s Origin-Private File System (OPFS) and perform arbitrary read/write/delete operations.

Root Cause Analysis

In content/browser/file_system_access/file_system_access_manager_impl.cc, the GetSandboxedFileSystem method is implemented as follows:

void FileSystemAccessManagerImpl::GetSandboxedFileSystem(
    const BindingContext& binding_context,
    const std::optional<storage::BucketLocator>& bucket,
    const std::vector<std::string>& directory_path_components,
    GetSandboxedFileSystemCallback callback) {
  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
  ...
  GetIOThreadTaskRunner({})->PostTask(
      FROM_HERE, base::BindOnce(&FileSystemContext::OpenFileSystem, context(),
                                binding_context.storage_key, bucket,
                                storage::kFileSystemTypeTemporary,
                                storage::OPEN_FILE_SYSTEM_CREATE_IF_NONEXISTENT,
                                std::move(response_callback)));
}

This method processes requests directly and opens the sandboxed filesystem without invoking ChildProcessSecurityPolicy validations on the requesting process ID (binding_context.process_id()). When the directory handle is successfully opened, GetSharedHandleStateForSandboxedPath() automatically returns a SharedHandleState with permissions hardcoded to PermissionStatus::GRANTED (lines 1974-1976).

Potential Attack / Trigger Steps

  1. A user loads a PDF document from a target origin (e.g., https://victim.example/document.pdf). The browser process isolates this document in a PDF renderer process locked to https://victim.example.
  2. An attacker exploits a potential vulnerability within the sandboxed PDF renderer process to achieve code execution.
  3. The compromised renderer requests a handle to the host origin’s OPFS through one of two potential paths:
    • Path A (Direct Binding): Binds the blink.mojom.FileSystemAccessManager interface and invokes GetSandboxedFileSystem().
    • Path B (Storage Buckets): Binds blink.mojom.BucketManagerHost and calls BucketHost::GetDirectory(), which routes to RenderFrameHostImpl::GetSandboxedFileSystemForBucket and directly calls the private overload of FileSystemAccessManagerImpl::GetSandboxedFileSystem.
  4. The browser opens the sandboxed file system for https://victim.example and returns a directory handle with always-granted permissions.
  5. The attacker can now read, write, create, or delete any files/databases inside the target origin’s OPFS.

Note: The steps above describe a potential exploit scenario. Our static analysis tooling does not currently have the capability to run code or dynamically verify this vulnerability with a working proof of concept.

Suggested Fix

To resolve this issue, add a validation step in FileSystemAccessManagerImpl::GetSandboxedFileSystem to ensure that the calling process is authorized to access storage for the requested origin using ChildProcessSecurityPolicyImpl::CanAccessDataForOrigin:

if (!ChildProcessSecurityPolicy::GetInstance()->CanAccessDataForOrigin(
        binding_context.process_id(), binding_context.storage_key.origin())) {
  std::move(callback).Run(
      file_system_access_error::FromFileError(base::File::FILE_ERROR_SECURITY),
      mojo::NullRemote());
  return;
}

Evaluated with Chrome root at commit: b1520ef4a76878853a31f0943b565e42060edec8

Results so far have been promising, but there can be wrong deductions. Feel free to adjust as follows:

  • If you are familiar with the severity guidelines, you may adjust the severity.
  • If this is a false positive, and there’s no work to be done, please close as WAI.
  • If there is work to do here but not a vulnerability, please change the issue type to Task/Bug/FR.

Data from false positives will be used to improve accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.

View on issue tracker
On This Page