CVE-2026-87493
Overview
Files Changed
chrome/browser/file_system_access/chrome_file_system_access_permission_context_browsertest.cc
Patch
From 2f959d402954f5e0fd2bcbcd58d973fbd8bcf9c0 Mon Sep 17 00:00:00 2001 From: Rahul Singh (EDGE) <[email protected]> Date: Thu, 06 Aug 2026 08:02:32 -0700 Subject: [PATCH] FSA: Source must be readable to move to readable dir Currently, move() gates the source handle on write permission only. However, when the destination directory is readable, a moved entry becomes readable through that directory's read grant. So, when moving into a readable directory, the source must also hold read permission. Otherwise a site could move a file it cannot read into a directory it can read. The moved file inherits that directory's read grant, allowing the site to bypass a read revocation and access the file's contents. This change ensures that moving a file or a directory with a revoked read grant (e.g. when remove() revokes a handle's read grant) into a readable directory fails. remove() revokes the read grant while keeping write. If the file or directory is recreated externally, the site's read grant is not restored. Without this fix, move() would let a site regain read access by relocating the entry into a readable directory. Moving into a write-only directory is unaffected, since the entry cannot become readable there. Bug: 523741272 Change-Id: Ia1f52033ac9269d29120a7ec17c98f5d5b35eb29 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8122214 Reviewed-by: Ming-Ying Chung <[email protected]> Commit-Queue: Rahul Singh <[email protected]> Cr-Commit-Position: refs/heads/main@{#1674979} --- diff --git a/chrome/browser/file_system_access/chrome_file_system_access_permission_context_browsertest.cc b/chrome/browser/file_system_access/chrome_file_system_access_permission_context_browsertest.cc index 36eabb02..5003933 100644 --- a/chrome/browser/file_system_access/chrome_file_system_access_permission_context_browsertest.cc +++ b/chrome/browser/file_system_access/chrome_file_system_access_permission_context_browsertest.cc @@ -1001,6 +1001,178 @@ ui::SelectFileDialog::SetFactory(nullptr); } +// Verifies that once remove() revokes a file handle's read grant, moving the +// handle into a directory the site can read is denied. Otherwise the site could +// read an externally-recreated file via the destination directory's read grant, +// bypassing the revocation. Regression test for crbug.com/523741272. +IN_PROC_BROWSER_TEST_F( + ChromeFileSystemAccessPermissionContextRevokeAndRestoreBrowserTest, + MoveOfRevokedHandleIntoReadableDirectoryIsDenied) { + // Navigate to a test page. + const GURL url = embedded_test_server()->GetURL("/empty.html"); + ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url)); + + const url::Origin origin = GetOrigin(); + + // Grant the origin Extended Permission so the later showDirectoryPicker() + // call does not wipe the file handle's dormant write grant. + permission_context()->SetOriginHasExtendedPermissionForTesting(origin); + + // Get a read/write handle to a file. + const base::FilePath test_file_path = CreateTestFile("test file contents"); + SetUpAndGetHandleWithInitialPermissions("handle", test_file_path, + /*expect_extended_grants=*/true); + + // Remove the file. + RemoveFileAndVerifyPermissionsRevoked("handle", origin, test_file_path, + /*expect_extended_write=*/true); + + // Simulate an external application recreating the file with sensitive data + // that the site is no longer authorized to read. + { + base::ScopedAllowBlockingForTesting allow_blocking; + ASSERT_TRUE(base::WriteFile(test_file_path, "sensitive external data")); + } + + // Get a read/write handle to a separate directory the site controls. Moving + // the file here would expose it via this directory's read grant. + base::FilePath dest_dir_path; + { + base::ScopedAllowBlockingForTesting allow_blocking; + ASSERT_TRUE(base::CreateTemporaryDirInDir( + temp_dir().GetPath(), FILE_PATH_LITERAL("dest"), &dest_dir_path)); + } + ui::SelectFileDialog::SetFactory( + std::make_unique<content::FakeSelectFileDialogFactory>( + std::vector<base::FilePath>{dest_dir_path})); + ASSERT_TRUE(content::ExecJs(GetWebContents(), R"((async () => { + self.dirHandle = await self.showDirectoryPicker({mode: 'readwrite'}); + })())")); + + // Moving the revoked handle into that readable directory must be denied, + // since the source handle no longer has read permission. + EXPECT_EQ("NotAllowedError", + content::EvalJs(GetWebContents(), R"((async () => { + try { + await self.handle.move(self.dirHandle); + return 'moved'; + } catch (e) { + return e.name; + } + })())")); + + // The file must not have been relocated into the readable directory. + { + base::ScopedAllowBlockingForTesting allow_blocking; + EXPECT_TRUE(base::PathExists(test_file_path)); + EXPECT_FALSE( + base::PathExists(dest_dir_path.Append(test_file_path.BaseName()))); + } + + // Read permission for the removed path must remain revoked. + VerifyPermissions(origin, test_file_path, + ChromeFileSystemAccessPermissionContext::HandleType::kFile, + content::PermissionStatus::DENIED, + content::PermissionStatus::GRANTED, + /*expected_extended_read=*/false, + /*expected_extended_write=*/true); + + ui::SelectFileDialog::SetFactory(nullptr); +} + +// Verifies that after remove() revokes a directory handle's read grant, an +// external application recreating the directory (with contents) does not +// restore read access. The site loses read access to both the directory and any +// file inside it, while retaining only its write grant. See +// crbug.com/523741272. +IN_PROC_BROWSER_TEST_F( + ChromeFileSystemAccessPermissionContextRevokeAndRestoreBrowserTest, + RemovedDirectoryRecreatedExternallyKeepsReadRevoked) { + // Navigate to a test page. + const GURL url = embedded_test_server()->GetURL("/empty.html"); + ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url)); + + const url::Origin origin = GetOrigin(); + + // Grant the origin Extended Permission so the picker call below does not wipe + // pre-existing grants. See `RestoreReadOnWrite_Move` for details. + permission_context()->SetOriginHasExtendedPermissionForTesting(origin); + + // Create a directory with no readable ancestor grant (so removing it actually + // revokes read) and obtain a read/write handle to it. + base::FilePath test_dir_path; + { + base::ScopedAllowBlockingForTesting allow_blocking; + ASSERT_TRUE(base::CreateTemporaryDirInDir( + temp_dir().GetPath(), FILE_PATH_LITERAL("target"), &test_dir_path)); + } + ui::SelectFileDialog::SetFactory( + std::make_unique<content::FakeSelectFileDialogFactory>( + std::vector<base::FilePath>{test_dir_path})); + FileSystemAccessPermissionRequestManager::FromWebContents(GetWebContents()) + ->set_auto_response_for_test(permissions::PermissionAction::GRANTED); + ASSERT_TRUE(content::ExecJs(GetWebContents(), R"((async () => { + self.dirHandle = await self.showDirectoryPicker({mode: 'readwrite'}); + })())")); + + // Verify initial read/write permissions are granted to the directory. + VerifyPermissions( + origin, test_dir_path, + ChromeFileSystemAccessPermissionContext::HandleType::kDirectory, + content::PermissionStatus::GRANTED, content::PermissionStatus::GRANTED, + /*expected_extended_read=*/true, /*expected_extended_write=*/true); + + // Remove the directory. Read permission is revoked while write is retained. + ASSERT_TRUE(content::ExecJs(GetWebContents(), R"((async () => { + await self.dirHandle.remove({recursive: true}); + })())")); + VerifyPermissions( + origin, test_dir_path, + ChromeFileSystemAccessPermissionContext::HandleType::kDirectory, + content::PermissionStatus::DENIED, content::PermissionStatus::GRANTED, + /*expected_extended_read=*/false, /*expected_extended_write=*/true); + ASSERT_TRUE(permission_context()->IsPathInDowngradedReadPathsForTesting( + origin, test_dir_path)); + + // Simulate an external application recreating the directory with a file + // inside it that the site is not authorized to read. + const base::FilePath secret_file = test_dir_path.AppendASCII("secret.txt"); + { + base::ScopedAllowBlockingForTesting allow_blocking; + ASSERT_TRUE(base::CreateDirectory(test_dir_path)); + ASSERT_TRUE(base::WriteFile(secret_file, "secret contents")); + } + + // External recreation must not restore read. It stays revoked and the path + // stays downgraded, while the write grant is retained. + VerifyPermissions( + origin, test_dir_path, + ChromeFileSystemAccessPermissionContext::HandleType::kDirectory, + content::PermissionStatus::DENIED, content::PermissionStatus::GRANTED, + /*expected_extended_read=*/false, /*expected_extended_write=*/true); + EXPECT_TRUE(permission_context()->IsPathInDowngradedReadPathsForTesting( + origin, test_dir_path)); + + // The revocation is observable to the site via the existing handle. + EXPECT_EQ("denied", content::EvalJs(GetWebContents(), R"((async () => { + return await self.dirHandle.queryPermission({mode: 'read'}); + })())")); + + // The site also cannot reach the externally-planted file through the + // revoked directory handle. Obtaining a child handle requires read access. + EXPECT_EQ("NotAllowedError", + content::EvalJs(GetWebContents(), R"((async () => { + try {
Regression Test / PoC
diff --git a/chrome/browser/file_system_access/chrome_file_system_access_permission_context_browsertest.cc b/chrome/browser/file_system_access/chrome_file_system_access_permission_context_browsertest.cc
index 36eabb02..5003933 100644
--- a/chrome/browser/file_system_access/chrome_file_system_access_permission_context_browsertest.cc
+++ b/chrome/browser/file_system_access/chrome_file_system_access_permission_context_browsertest.cc
@@ -1001,6 +1001,178 @@
ui::SelectFileDialog::SetFactory(nullptr);
}
+// Verifies that once remove() revokes a file handle's read grant, moving the
+// handle into a directory the site can read is denied. Otherwise the site could
+// read an externally-recreated file via the destination directory's read grant,
+// bypassing the revocation. Regression test for crbug.com/523741272.
+IN_PROC_BROWSER_TEST_F(
+ ChromeFileSystemAccessPermissionContextRevokeAndRestoreBrowserTest,
+ MoveOfRevokedHandleIntoReadableDirectoryIsDenied) {
+ // Navigate to a test page.
+ const GURL url = embedded_test_server()->GetURL("/empty.html");
+ ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url));
+
+ const url::Origin origin = GetOrigin();
+
+ // Grant the origin Extended Permission so the later showDirectoryPicker()
+ // call does not wipe the file handle's dormant write grant.
+ permission_context()->SetOriginHasExtendedPermissionForTesting(origin);
+
+ // Get a read/write handle to a file.
+ const base::FilePath test_file_path = CreateTestFile("test file contents");
+ SetUpAndGetHandleWithInitialPermissions("handle", test_file_path,
+ /*expect_extended_grants=*/true);
+
+ // Remove the file.
+ RemoveFileAndVerifyPermissionsRevoked("handle", origin, test_file_path,
+ /*expect_extended_write=*/true);
+
+ // Simulate an external application recreating the file with sensitive data
+ // that the site is no longer authorized to read.
+ {
+ base::ScopedAllowBlockingForTesting allow_blocking;
+ ASSERT_TRUE(base::WriteFile(test_file_path, "sensitive external data"));
+ }
+
+ // Get a read/write handle to a separate directory the site controls. Moving
+ // the file here would expose it via this directory's read grant.
+ base::FilePath dest_dir_path;
+ {
+ base::ScopedAllowBlockingForTesting allow_blocking;
+ ASSERT_TRUE(base::CreateTemporaryDirInDir(
+ temp_dir().GetPath(), FILE_PATH_LITERAL("dest"), &dest_dir_path));
+ }
+ ui::SelectFileDialog::SetFactory(
+ std::make_unique<content::FakeSelectFileDialogFactory>(
+ std::vector<base::FilePath>{dest_dir_path}));
+ ASSERT_TRUE(content::ExecJs(GetWebContents(), R"((async () => {
+ self.dirHandle = await self.showDirectoryPicker({mode: 'readwrite'});
+ })())"));
+
+ // Moving the revoked handle into that readable directory must be denied,
+ // since the source handle no longer has read permission.
+ EXPECT_EQ("NotAllowedError",
+ content::EvalJs(GetWebContents(), R"((async () => {
+ try {
+ await self.handle.move(self.dirHandle);
+ return 'moved';
+ } catch (e) {
+ return e.name;
+ }
+ })())"));
+
+ // The file must not have been relocated into the readable directory.
+ {
+ base::ScopedAllowBlockingForTesting allow_blocking;
+ EXPECT_TRUE(base::PathExists(test_file_path));
+ EXPECT_FALSE(
+ base::PathExists(dest_dir_path.Append(test_file_path.BaseName())));
+ }
+
+ // Read permission for the removed path must remain revoked.
+ VerifyPermissions(origin, test_file_path,
+ ChromeFileSystemAccessPermissionContext::HandleType::kFile,
+ content::PermissionStatus::DENIED,
+ content::PermissionStatus::GRANTED,
+ /*expected_extended_read=*/false,
+ /*expected_extended_write=*/true);
+
+ ui::SelectFileDialog::SetFactory(nullptr);
+}
+
+// Verifies that after remove() revokes a directory handle's read grant, an
+// external application recreating the directory (with contents) does not
+// restore read access. The site loses read access to both the directory and any
+// file inside it, while retaining only its write grant. See
+// crbug.com/523741272.
+IN_PROC_BROWSER_TEST_F(
+ ChromeFileSystemAccessPermissionContextRevokeAndRestoreBrowserTest,
+ RemovedDirectoryRecreatedExternallyKeepsReadRevoked) {
+ // Navigate to a test page.
+ const GURL url = embedded_test_server()->GetURL("/empty.html");
+ ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url));
+
+ const url::Origin origin = GetOrigin();
+
+ // Grant the origin Extended Permission so the picker call below does not wipe
+ // pre-existing grants. See `RestoreReadOnWrite_Move` for details.
+ permission_context()->SetOriginHasExtendedPermissionForTesting(origin);
+
+ // Create a directory with no readable ancestor grant (so removing it actually
+ // revokes read) and obtain a read/write handle to it.
+ base::FilePath test_dir_path;
+ {
+ base::ScopedAllowBlockingForTesting allow_blocking;
+ ASSERT_TRUE(base::CreateTemporaryDirInDir(
+ temp_dir().GetPath(), FILE_PATH_LITERAL("target"), &test_dir_path));
+ }
+ ui::SelectFileDialog::SetFactory(
+ std::make_unique<content::FakeSelectFileDialogFactory>(
+ std::vector<base::FilePath>{test_dir_path}));
+ FileSystemAccessPermissionRequestManager::FromWebContents(GetWebContents())
+ ->set_auto_response_for_test(permissions::PermissionAction::GRANTED);
+ ASSERT_TRUE(content::ExecJs(GetWebContents(), R"((async () => {
+ self.dirHandle = await self.showDirectoryPicker({mode: 'readwrite'});
+ })())"));
+
+ // Verify initial read/write permissions are granted to the directory.
+ VerifyPermissions(
+ origin, test_dir_path,
+ ChromeFileSystemAccessPermissionContext::HandleType::kDirectory,
+ content::PermissionStatus::GRANTED, content::PermissionStatus::GRANTED,
+ /*expected_extended_read=*/true, /*expected_extended_write=*/true);
+
+ // Remove the directory. Read permission is revoked while write is retained.
+ ASSERT_TRUE(content::ExecJs(GetWebContents(), R"((async () => {
+ await self.dirHandle.remove({recursive: true});
+ })())"));
+ VerifyPermissions(
+ origin, test_dir_path,
+ ChromeFileSystemAccessPermissionContext::HandleType::kDirectory,
+ content::PermissionStatus::DENIED, content::PermissionStatus::GRANTED,
+ /*expected_extended_read=*/false, /*expected_extended_write=*/true);
+ ASSERT_TRUE(permission_context()->IsPathInDowngradedReadPathsForTesting(
+ origin, test_dir_path));
+
+ // Simulate an external application recreating the directory with a file
+ // inside it that the site is not authorized to read.
+ const base::FilePath secret_file = test_dir_path.AppendASCII("secret.txt");
+ {
+ base::ScopedAllowBlockingForTesting allow_blocking;
+ ASSERT_TRUE(base::CreateDirectory(test_dir_path));
+ ASSERT_TRUE(base::WriteFile(secret_file, "secret contents"));
+ }
+
+ // External recreation must not restore read. It stays revoked and the path
+ // stays downgraded, while the write grant is retained.
+ VerifyPermissions(
+ origin, test_dir_path,
+ ChromeFileSystemAccessPermissionContext::HandleType::kDirectory,
+ content::PermissionStatus::DENIED, content::PermissionStatus::GRANTED,
+ /*expected_extended_read=*/false, /*expected_extended_write=*/true);
+ EXPECT_TRUE(permission_context()->IsPathInDowngradedReadPathsForTesting(
+ origin, test_dir_path));
+
+ // The revocation is observable to the site via the existing handle.
+ EXPECT_EQ("denied", content::EvalJs(GetWebContents(), R"((async () => {
+ return await self.dirHandle.queryPermission({mode: 'read'});
+ })())"));
+
+ // The site also cannot reach the externally-planted file through the
+ // revoked directory handle. Obtaining a child handle requires read access.
+ EXPECT_EQ("NotAllowedError",
+ content::EvalJs(GetWebContents(), R"((async () => {
+ try {
+ await self.dirHandle.getFileHandle('secret.txt');
+ return 'got handle';
+ } catch (e) {
+ return e.name;
+ }
+ })())"));
+
+ ui::SelectFileDialog::SetFactory(nullptr);
+}
+
// Tests that after a file is renamed to the removed file path, the read
// permission for that file path is restored.
IN_PROC_BROWSER_TEST_F(
diff --git a/content/browser/file_system_access/file_system_access_directory_handle_impl_unittest.cc b/content/browser/file_system_access/file_system_access_directory_handle_impl_unittest.cc
index 7556b8d7..e87297f 100644
--- a/content/browser/file_system_access/file_system_access_directory_handle_impl_unittest.cc
+++ b/content/browser/file_system_access/file_system_access_directory_handle_impl_unittest.cc
@@ -30,6 +30,7 @@
#include "content/browser/file_system_access/mock_file_system_access_permission_grant.h"
#include "content/public/browser/file_system_access_permission_context.h"
#include "content/public/test/browser_task_environment.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/self_owned_receiver.h"
#include "storage/browser/quota/quota_manager_proxy.h"
#include "storage/browser/test/test_file_system_context.h"
@@ -564,6 +565,34 @@
}
#endif // BUILDFLAG(IS_ANDROID)
+// A write-only source directory handle (e.g. one whose read grant was revoked
+// by remove()) must not be moved into a directory the site can read.
+// See crbug.com/523741272.
+TEST_F(FileSystemAccessDirectoryHandleImplTest, MoveNoReadAccess) {
+ base::FilePath source_dir = dir_.GetPath().AppendASCII("source");
+ ASSERT_TRUE(base::CreateDirectory(source_dir));
+ base::FilePath dest_dir = dir_.GetPath().AppendASCII("dest");
+ ASSERT_TRUE(base::CreateDirectory(dest_dir));
+ base::FilePath moved_dir = dest_dir.AppendASCII("moved");
+
+ auto dest_dir_handle =
+ GetHandleWithPermissions(dest_dir, /*read=*/true, /*write=*/true);
+ auto handle =
+ GetHandleWithPermissions(source_dir, /*read=*/false, /*write=*/true);
+
+ mojo::PendingRemote<blink::mojom::FileSystemAccessTransferToken> dest_token;
+ manager_->CreateTransferToken(*dest_dir_handle,
+ dest_token.InitWithNewPipeAndPassReceiver());
+
+ base::test::TestFuture<blink::mojom::FileSystemAccessErrorPtr> future;
+ handle->Move(std::move(dest_token), moved_dir.BaseName().AsUTF8Unsafe(),
+ future.GetCallback());
+ EXPECT_EQ(future.Get()->status,
+ blink::mojom::FileSystemAccessStatus::kPermissionDenied);
+ EXPECT_TRUE(base::DirectoryExists(source_dir));
+ EXPECT_FALSE(base::DirectoryExists(moved_dir));
+}
+
// Tests for `FileSystemAccessDirectoryHandleImpl::Remove()`.
class FileSystemAccessDirectoryHandleImplRemoveTest
: public FileSystemAccessDirectoryHandleImplPermissionTestBase,
diff --git a/content/browser/file_system_access/file_system_access_file_handle_impl_browsertest.cc b/content/browser/file_system_access/file_system_access_file_handle_impl_browsertest.cc
index a3bed4e..3c35aa04 100644
--- a/content/browser/file_system_access/file_system_access_file_handle_impl_browsertest.cc
+++ b/content/browser/file_system_access/file_system_access_file_handle_impl_browsertest.cc
@@ -204,15 +204,15 @@
"})()"));
}
-// Verifies that `move()` on a local FS requests the correct permission mode
-// depending on whether the `kFileSystemAccessWriteMode` feature is enabled.
+// Verifies that `move()` on a local FS requests read-write permission on the
+// source, regardless of the `kFileSystemAccessWriteMode` feature.
IN_PROC_BROWSER_TEST_P(FileSystemAccessFileHandleImplWriteModeBrowserTest,
Local_Move_RequestsCorrectPermissions) {
CreateTestFileAndDirectory(temp_dir_.GetPath(), "test file");
// Calling the above setup method creates two shared handle states.
ExpectGetPermissionStatusAndReturnGranted(
- GetParam().expected_mode,
+ FileSystemAccessPermissionMode::kReadWrite,
/*expected_shared_handle_state_count=*/2u);
EXPECT_TRUE(ExecJs(shell(), R"((async () => {
@@ -220,9 +220,8 @@
})())"));
}
-// Verifies that `move()` on a sandboxed FS requests the correct permission
-// mode depending on whether the `kFileSystemAccessWriteMode` feature is
-// enabled.
+// Verifies that `move()` on a sandboxed FS requests read-write permission on
+// the source, regardless of the `kFileSystemAccessWriteMode` feature.
IN_PROC_BROWSER_TEST_P(FileSystemAccessFileHandleImplWriteModeBrowserTest,
Sandboxed_Move_RequestsCorrectPermissions) {
ASSERT_TRUE(NavigateToURL(shell(), test_url_));
@@ -235,7 +234,8 @@
await writable.close();
})())"));
- ExpectGetPermissionStatusAndReturnGranted(GetParam().expected_mode);
+ ExpectGetPermissionStatusAndReturnGranted(
+ FileSystemAccessPermissionMode::kReadWrite);
EXPECT_TRUE(ExecJs(shell(), R"((async () => {
await self.fileHandle.move(self.sandboxDir);
diff --git a/content/browser/file_system_access/file_system_access_file_handle_impl_unittest.cc b/content/browser/file_system_access/file_system_access_file_handle_impl_unittest.cc
index 0880dd8..b91d4f6 100644
--- a/content/browser/file_system_access/file_system_access_file_handle_impl_unittest.cc
+++ b/content/browser/file_system_access/file_system_access_file_handle_impl_unittest.cc
@@ -920,6 +920,63 @@
EXPECT_TRUE(base::PathExists(renamed_file));
}
+// A write-only source handle (e.g. one whose read grant was revoked by
+// remove()) must not be moved into a directory the site can read.
+// See crbug.com/523741272.
+TEST_F(FileSystemAccessFileHandleImplMoveTest, NoReadAccess) {
+ base::FilePath dest_dir;
+ ASSERT_TRUE(base::CreateTemporaryDirInDir(
+ dir_.GetPath(), FILE_PATH_LITERAL("dest"), &dest_dir));
+ base::FilePath file;
+ ASSERT_TRUE(base::CreateTemporaryFileInDir(dir_.GetPath(), &file));
+ base::FilePath renamed_file = dest_dir.AppendASCII("new_name.txt");
+
+ auto dest_dir_handle =
+ GetDirectoryHandleWithPermissions(dest_dir, /*read_grant=*/allow_grant_,
+ /*write_grant=*/allow_grant_);
+ auto handle = GetHandleWithPermissions(file, /*read_grant=*/deny_grant_,
+ /*write_grant=*/allow_grant_);
+
... (truncated)
Original Bug Report
Potential bypass of kFileSystemAccessRevokeReadOnRemove via Move operation
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: The File System Access API can potentially bypass the read permission revocation intended by kFileSystemAccessRevokeReadOnRemove. When a site deletes a file, its read grant is revoked, but its write grant is retained. The site can then move an externally recreated version of that file into a directory it controls, acquiring a new handle that inherits the directory’s read access and exposing the file’s contents.
Affected files:
content/browser/file_system_access/file_system_access_file_handle_impl.cccontent/browser/file_system_access/file_system_access_handle_base.cccontent/browser/file_system_access/file_system_access_directory_handle_impl.cc
Estimated timestamp from git blame: Unknown (Google3 checkout)
Summary
A potential security bypass exists in the File System Access API’s kFileSystemAccessRevokeReadOnRemove mechanism. This feature is designed to downgrade read access to a file handle when the file is removed. However, because the move() operation on a file handle only checks for write permissions, an attacker can relocate a file with revoked read access into a directory for which they have full read/write access. By then retrieving a new handle to the moved file from the destination directory, the site gains read access via directory-based permission inheritance, bypassing the original revocation.
Technical Details
When a file is removed via FileSystemHandle.remove(), the FileSystemAccessHandleBase::DidRemove callback triggers NotifyEntryRemoved if the kFileSystemAccessRevokeReadOnRemove feature is active. ChromeFileSystemAccessPermissionContext::NotifyEntryRemoved specifically downgrades the active and persistent read grants for that path. Crucially, the site’s write grant for the file handle is left completely intact.
If the file is subsequently recreated at the same path by an external application, the site can call fileHandle.move(dirHandle, 'new_name') on its existing handle. The implementation in content/browser/file_system_access/file_system_access_file_handle_impl.cc uses GetEffectiveWritePermissionMode() for the Move operation. Since the kFileSystemAccessWriteMode feature is stable, this evaluates to kWrite.
Therefore, RunWithPermission only verifies that the site has kWrite access to the source file, which it still does because only the read grant was revoked. The system does not verify if the site possesses kRead access to the source file. The file is successfully moved to the destination directory.
When the site calls getFileHandle() on the destination directory, the new file handle is created in FileSystemAccessDirectoryHandleImpl::DidGetFile using the directory’s handle_state(). Since handle_state() contains the directory’s granted permissions, the new handle to the moved file perfectly inherits the directory’s read access. This allows the site to read the sensitive data in the recreated file, circumventing the intended read revocation.
Potential Steps to Reproduce
Note: These steps are based on static analysis and have not been executed via a working exploit.
- Use a browser with the
FileSystemAccessRevokeReadOnRemoveandFileSystemAccessWriteModefeatures enabled (default in stable). - A site prompts the user to grant read/write access to a directory via
showDirectoryPicker(). - The site prompts the user to grant read/write access to a specific file via
showSaveFilePicker(). - The site calls
fileHandle.remove(). The backend revokes the read permission for this handle but leaves the write permission intact. - An external process (or user action outside the browser) recreates the file at the original path, containing sensitive data.
- The site calls
fileHandle.move(dirHandle, 'leaked.txt'). The operation succeeds because write access to the source is still held. - The site calls
const newHandle = await dirHandle.getFileHandle('leaked.txt'). - The site calls
await (await newHandle.getFile()).text(), gaining unauthorized access to the sensitive data.
Suggested Fix
When performing a Move operation across different directories (where the parent directory is not the same as the destination directory, or when moving from an explicit file grant to a directory grant), the system should verify that the site possesses both kRead and kWrite permissions for the source file, rather than just kWrite. Alternatively, if kFileSystemAccessRevokeReadOnRemove is intended to revoke all access to a deleted file, it should be updated to revoke both read and write grants.
Evaluated with Chrome root at commit: 65b3256311f3ab6fb9870eaa522de7e6dd2663bb
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.