Chrome · FileSystem › Blink>Storage>FileSystem
CVE-2026-84354
Authorization Error in FileSystem (Blink>Storage>FileSystem)
Overview
High
Severity
9.6
CVSS
No
Exploited ITW
Fixed
Fix Status
Background
- File System Access `move()`
- the
FileSystemFileHandle.move()API that renames/moves a file a web origin has a handle to; on non-OPFS (real local) files this touches the user’s actual disk. - OPFS / `kFileSystemTypeTemporary`
- the sandboxed Origin Private File System, where the origin always has full access and no user-visible files exist.
- Write grant / `SharedHandleState`
- the permission state (
PermissionStatus::GRANTEDor not) that awrite_grantreports for a given path, tracked viaGetSharedHandleStateForNonSandboxedPath. - Transient user activation
- a short-lived flag indicating a recent user gesture, previously used as the fallback authorization for a rename.
Root Cause Analysis
Before the patch, DoRename authorized a non-OPFS rename with only two conditions: write access to the destination file itself, or, failing that, a transient user activation — the removed comment states outright “Write access to the parent directory is not required for renames.” That meant a page holding a write handle to file A, with a user gesture, could rename/move it to an arbitrary destination path (e.g. a different name or location in the directory) without ever having write permission to the parent directory that governs creating/removing entries there, violating the expectation that creating a new named entry in a directory requires write access to that directory.
The fix introduces GetRenamePermission, which returns kAllowOverwrite only for OPFS or when the destination file’s write grant is GRANTED; otherwise, under the new (flag-guarded) behavior it requires the destination’s parent directory write grant to be GRANTED (optionally only when the destination is inside the home directory, per kOnlyInHomedir), returning kDeny when that permission is absent instead of allowing a gesture bypass. DoRename now consults GetRenamePermission and denies with kPermissionDenied on kDeny, and the former has_write_access_to_destination boolean is renamed to has_overwrite_permission threaded through PrepareForMove → DidTakeMoveLocks → DidVerifySensitiveEntryAccessForMove, which still only performs an overwriting move when that flag is set. The change is a tightening: the legacy gesture-only path survives only behind the disabled-by-default feature flag or outside the home directory, and the regression test Rename_NoParentWriteAccessFails confirms a rename now fails with NotAllowedError without parent write access.
Key insight
Renaming/moving a real file only checked write access to the target file (or a user gesture), never write access to the parent directory where the new entry is created — so a page could place a file at an attacker-chosen path it was never authorized to write; the fix requires parent-directory write permission (via the new
GetRenamePermission) unless the destination file itself is already writable.Attack Path
- Obtain a file handle
A page acquires a writable
FileSystemFileHandleto one local file the user granted (e.g. viashowOpenFilePicker), giving it write access to that single file but not to its containing directory. - Invoke `move()` with a gesture
Within a user gesture, the page calls
handle.move(newName)to rename/relocate the file to a different destination path in the directory to which it has no directory-level write grant. - Pre-patch authorization passes
DoRenamesees no destination write grant but accepts the transient user activation, treating the rename as authorized and proceeding toPrepareForMove, so the file is created/removed at a path the origin was never granted directory write access to. - Impact / fix
This lets the origin write a new named entry into a directory it lacks permission to (incorrect authorization over the file system)
(inferred)for any specific sensitive-path abuse; after the patchGetRenamePermissionreturnskDenyfor that case andmove()rejects withNotAllowedError.
Impact Assessment
The advisory rates this as incorrect authorization in the File System Access component; an attacker web origin holding a write handle to a single file (plus, pre-patch, a user gesture) could rename/move it into a parent directory for which it had no directory write permission, i.e. exceed the scope of its granted file-system permission. Exploitation requires the user to have granted a file handle and to trigger the gesture; the fix is disabled-by-default (feature-flag guarded, optionally home-directory-only). No memory-safety or code-execution consequence is demonstrated, and any concrete abuse of a sensitive destination path is
(inferred).Changed Functions
| Function | Change | Notes |
|---|---|---|
ifcontent/browser/file_system_access/file_system_access_handle_base.cc |
modified | |
TestFileSystemAccessPermissionContextchrome/browser/file_system_access/chrome_file_system_access_permission_context_browsertest.cc |
modified |
Files Changed
content/browser/file_system_access/file_system_access_handle_base_unittest.cccontent/browser/file_system_access/features.hcontent/browser/file_system_access/file_system_access_file_handle_impl_unittest.ccchrome/browser/file_system_access/chrome_file_system_access_permission_context_browsertest.cccontent/browser/file_system_access/features.cccontent/browser/file_system_access/file_system_access_handle_base.hcontent/browser/file_system_access/file_system_access_handle_base.cc
Audit Directions
- Create/rename/move without directory-level authorizationAudit File System Access operations that create or relocate named entries and confirm they check write permission on the containing directory, not just the file being moved.
- User gesture used as a permission substituteSearch for authorization paths that accept transient user activation in lieu of an actual permission grant; a gesture is not a grant to a new resource.
- OPFS-vs-local branchingReview code that special-cases
kFileSystemTypeTemporary/OPFS to ensure the non-sandboxed local path applies the stricter real-disk permission checks and does not inherit OPFS’s implicit-access assumptions.
Patch
From a4775d86854c8f308550f5d5be2e88d8f48646e3 Mon Sep 17 00:00:00 2001 From: Yuzu Saijo <[email protected]> Date: Wed, 17 Jun 2026 18:57:49 -0700 Subject: [PATCH] Require write access to parent directory for rename This CL changes the rename operation on non-OPFS files using `FileSystemFileHandle.move()` to require its parent directory write access. The exception is when renaming file A to file B, where we already have write access to file B. This change is flag-guarded, and the param controls whether we enforce parent directory write access only in home directory or everywhere. Before the change, rename operation required only file write access or user gesture activation, and this was a security risk. Bug:498839176 Change-Id: I8992b728501efa2c2908a31670ff75883a5a0f2e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7867458 Commit-Queue: Yuzu Saijo <[email protected]> Reviewed-by: Fergal Daly <[email protected]> Cr-Commit-Position: refs/heads/main@{#1648752} --- diff --git a/content/browser/file_system_access/features.cc b/content/browser/file_system_access/features.cc index a365deee3..2cc9e3c 100644 --- a/content/browser/file_system_access/features.cc +++ b/content/browser/file_system_access/features.cc @@ -68,4 +68,15 @@ 2 << 28 // 1/2GiB ); +// When enabled, rename in the local file system requires write permission to +// its parent directory. +BASE_FEATURE(kFileSystemAccessRenameRequiresParentWritePermission, + base::FEATURE_DISABLED_BY_DEFAULT); + +// Controls whether the rename permission check is restricted to the home +// directory. +BASE_FEATURE_PARAM(bool, + kOnlyInHomedir, + &kFileSystemAccessRenameRequiresParentWritePermission, + true); } // namespace content::features diff --git a/content/browser/file_system_access/features.h b/content/browser/file_system_access/features.h index 530c710..4bf18e59 100644 --- a/content/browser/file_system_access/features.h +++ b/content/browser/file_system_access/features.h @@ -33,7 +33,9 @@ kFileSystemObserverQuotaLimitMacPercent); CONTENT_EXPORT BASE_DECLARE_FEATURE_PARAM(size_t, kFileSystemObserverQuotaLimitWindows); - +CONTENT_EXPORT BASE_DECLARE_FEATURE( + kFileSystemAccessRenameRequiresParentWritePermission); +CONTENT_EXPORT BASE_DECLARE_FEATURE_PARAM(bool, kOnlyInHomedir); } // namespace content::features #endif // CONTENT_BROWSER_FILE_SYSTEM_ACCESS_FEATURES_H_ diff --git a/content/browser/file_system_access/file_system_access_handle_base.cc b/content/browser/file_system_access/file_system_access_handle_base.cc index 4a6555d6..2571f18 100644 --- a/content/browser/file_system_access/file_system_access_handle_base.cc +++ b/content/browser/file_system_access/file_system_access_handle_base.cc @@ -8,10 +8,12 @@ #include <vector> #include "base/barrier_callback.h" +#include "base/base_paths.h" #include "base/feature_list.h" #include "base/files/file_path.h" #include "base/files/safe_base_name.h" #include "base/functional/bind.h" +#include "base/path_service.h" #include "base/strings/strcat.h" #include "base/strings/utf_string_conversions.h" #include "content/browser/file_system_access/features.h" @@ -297,13 +299,100 @@ std::move(callback))); } +FileSystemAccessHandleBase::RenamePermission +FileSystemAccessHandleBase::GetRenamePermission( + const std::string& new_entry_name, + const bool has_transient_user_activation, + const storage::FileSystemURL destination_url) { + if (destination_url.type() == + storage::FileSystemType::kFileSystemTypeTemporary) { + // For OPFS files, rename with overwrite permission is always allowed. + return RenamePermission::kAllowOverwrite; + } + + // 1. Check write permission to the target file. + CHECK_NE(url().type(), storage::FileSystemType::kFileSystemTypeTemporary); + SharedHandleState destination_shared_handle_state = + manager()->GetSharedHandleStateForNonSandboxedPath( + content::PathInfo(destination_url.virtual_path(), new_entry_name), + context().storage_key, + // TODO(crbug.com/40198034): Support directory moves. + FileSystemAccessPermissionContext::HandleType::kFile, + FileSystemAccessPermissionContext::UserAction::kNone); + + if (destination_shared_handle_state.write_grant->GetStatus() == + PermissionStatus::GRANTED) { + // When write access to the target file is granted, rename with overwrite + // permission is allowed, regardless of the flag. + return RenamePermission::kAllowOverwrite; + } + + // 2. Determine if we need to check parent directory write permission. + // Legacy Behavior (flag off): allow gesture bypass. + if (!base::FeatureList::IsEnabled( + features::kFileSystemAccessRenameRequiresParentWritePermission)) { + return has_transient_user_activation + ? RenamePermission::kAllowRenameWithoutOverwrite + : RenamePermission::kDeny; + } + // New behavior (flag on): since we don't have write permission to the target + // file, write permission to the parent directory is required to rename the + // file. We no longer need to check user activation, because user activation + // was introduced as a fallback security mitigation instead of requiring + // parent directory write permission, in order to avoid silent renaming in the + // background. + bool should_check_parent_directory_permission = true; + // If `kOnlyInHomedir` is true, the parent directory write permission check is + // restricted to the home directory. + if (features::kOnlyInHomedir.Get()) { + // When the param is set to true, check the parent directory access only + // when the destination is in home directory. + base::FilePath home_dir; + if (base::PathService::Get(base::DIR_HOME, &home_dir)) { + base::FilePath parent_dir = destination_url.path().DirName(); + should_check_parent_directory_permission = + home_dir.IsParent(parent_dir) || (home_dir == parent_dir); + } + } + + // 3. Perform the appropriate permission check. + if (should_check_parent_directory_permission) { + SharedHandleState parent_shared_handle_state = + manager()->GetSharedHandleStateForNonSandboxedPath( + content::PathInfo( + destination_url.type() == storage::kFileSystemTypeExternal + ? PathType::kExternal + : PathType::kLocal, + destination_url.path().DirName()), + context().storage_key, + FileSystemAccessPermissionContext::HandleType::kDirectory, + FileSystemAccessPermissionContext::UserAction::kNone); + + if (parent_shared_handle_state.write_grant->GetStatus() == + PermissionStatus::GRANTED) { + // Parent directory permission is granted. + return RenamePermission::kAllowOverwrite; + } else { + return RenamePermission::kDeny; + } + } else { + // Flag on with `kOnlyHomedir` true, but the destination is not in the + // homedir. Allow gesture bypass. + CHECK(features::kOnlyInHomedir.Get()); + if (!has_transient_user_activation) { + return RenamePermission::kDeny; + } else { + return RenamePermission::kAllowRenameWithoutOverwrite; + } + } +} + void FileSystemAccessHandleBase::DoRename( const std::string& new_entry_name, bool has_transient_user_activation, base::OnceCallback<void(blink::mojom::FileSystemAccessErrorPtr)> callback) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); // To get this far, we must have write access to the entry being moved. - // Write access to the parent directory is not required for renames. DCHECK_EQ(GetEffectiveWritePermissionStatus(), blink::mojom::PermissionStatus::GRANTED); @@ -330,32 +419,18 @@ #endif CHECK(destination_url.is_valid()); - SharedHandleState destination_shared_handle_state = - url().type() == storage::FileSystemType::kFileSystemTypeTemporary - ? manager()->GetSharedHandleStateForSandboxedPath() - : manager()->GetSharedHandleStateForNonSandboxedPath( - content::PathInfo(destination_url.virtual_path(), - new_entry_name), - context().storage_key, - // TODO(crbug.com/40198034): Support directory moves. - FileSystemAccessPermissionContext::HandleType::kFile, - FileSystemAccessPermissionContext::UserAction::kNone); - - // Require a user gesture if the write access to the destination is not - // explicitly granted. - bool has_write_access_to_destination = - destination_shared_handle_state.write_grant->GetStatus() == - PermissionStatus::GRANTED; - if (!has_write_access_to_destination && !has_transient_user_activation) { - // Files in the OPFS always have write access and should never be gated on a - // user gesture requirement. + RenamePermission permission = GetRenamePermission( + new_entry_name, has_transient_user_activation, destination_url); + if (permission == RenamePermission::kDeny) { CHECK_NE(destination_url.type(), storage::kFileSystemTypeTemporary); std::move(callback).Run(file_system_access_error::FromStatus( blink::mojom::FileSystemAccessStatus::kPermissionDenied)); return; } - PrepareForMove(std::move(destination_url), has_write_access_to_destination, + const bool has_overwrite_permission = + permission == RenamePermission::kAllowOverwrite; + PrepareForMove(std::move(destination_url), has_overwrite_permission, has_transient_user_activation, std::move(callback)); } @@ -427,7 +502,7 @@ void FileSystemAccessHandleBase::PrepareForMove( storage::FileSystemURL destination_url, - bool has_write_access_to_destination, + bool has_overwrite_permission, bool has_transient_user_activation, base::OnceCallback<void(blink::mojom::FileSystemAccessErrorPtr)> callback) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); @@ -439,7 +514,7 @@ take_destination_lock ? 2 : 1, base::BindOnce(&FileSystemAccessHandleBase::DidTakeMoveLocks, AsWeakPtr(), destination_url, has_transient_user_activation, - has_write_access_to_destination, std::move(callback))); + has_overwrite_permission, std::move(callback))); manager()->TakeLock(context(), url(), manager()->GetExclusiveLockType(), barrier_callback); @@ -452,7 +527,7 @@ void FileSystemAccessHandleBase::DidTakeMoveLocks( storage::FileSystemURL destination_url, bool has_transient_user_activation, - bool has_write_access_to_destination, + bool has_overwrite_permission, base::OnceCallback<void(blink::mojom::FileSystemAccessErrorPtr)> callback, std::vector<scoped_refptr<LockHandle>> locks) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); @@ -497,16 +572,16 @@ context().frame_id, base::BindOnce( &FileSystemAccessHandleBase::DidVerifySensitiveEntryAccessForMove, - AsWeakPtr(), std::move(destination_url), - has_write_access_to_destination, has_transient_user_activation, - std::move(callback), std::move(locks))); + AsWeakPtr(), std::move(destination_url), has_overwrite_permission, + has_transient_user_activation, std::move(callback), + std::move(locks))); } else { // Skipping ConfirmSensitiveEntryAccess() as either of the following holds: // (1) no permission context. Possibly because manager() is being destroyed // or in a test. // (2) destination file is in Bucket File System, i.e. not a real file. DidVerifySensitiveEntryAccessForMove( - std::move(destination_url), has_write_access_to_destination, + std::move(destination_url), has_overwrite_permission, has_transient_user_activation, std::move(callback), std::move(locks), FileSystemAccessPermissionContext::SensitiveEntryResult::kAllowed); } @@ -514,7 +589,7 @@ void FileSystemAccessHandleBase::DidVerifySensitiveEntryAccessForMove( storage::FileSystemURL destination_url, - bool has_write_access_to_destination, + bool has_overwrite_permission, bool has_transient_user_activation, base::OnceCallback<void(blink::mojom::FileSystemAccessErrorPtr)> callback, std::vector<scoped_refptr<LockHandle>> locks, @@ -529,8 +604,9 @@ return; } - // Only allow overwriting moves if we have write access to the destination. - if (has_write_access_to_destination) { + // Only allow overwriting moves if we have write access to the destination or + // the parent directory write permission. + if (has_overwrite_permission) { DoPerformMoveOperation(destination_url, std::move(locks), has_transient_user_activation, std::move(callback)); return; diff --git a/content/browser/file_system_access/file_system_access_handle_base.h b/content/browser/file_system_access/file_system_access_handle_base.h index 3591fb4..b43a70c0 100644 --- a/content/browser/file_system_access/file_system_access_handle_base.h +++ b/content/browser/file_system_access/file_system_access_handle_base.h @@ -151,6 +151,17 @@ // See crbug.com/421690393. void MaybeNotifyEntryModified(const storage::FileSystemURL& url); + enum class RenamePermission { + kDeny, + kAllowOverwrite, + kAllowRenameWithoutOverwrite, + }; + + RenamePermission GetRenamePermission( + const std::string& new_entry_name, + const bool has_transient_user_activation, + const storage::FileSystemURL destination_url); + SEQUENCE_CHECKER(sequence_checker_); private: @@ -168,20 +179,20 @@ FileSystemAccessTransferTokenImpl* resolved_token); void PrepareForMove( storage::FileSystemURL destination_url, - bool has_write_access_to_destination, + bool has_overwrite_permission, bool has_transient_user_activation, base::OnceCallback<void(blink::mojom::FileSystemAccessErrorPtr)> callback); void DidTakeMoveLocks( storage::FileSystemURL destination_url, bool has_transient_user_activation, - bool has_write_access_to_destination, + bool has_overwrite_permission, base::OnceCallback<void(blink::mojom::FileSystemAccessErrorPtr)> callback, std::vector<scoped_refptr<FileSystemAccessLockManager::LockHandle>> locks); void DidVerifySensitiveEntryAccessForMove( storage::FileSystemURL destination_url, - bool has_write_access_to_destination, + bool has_overwrite_permission, bool has_transient_user_activation, base::OnceCallback<void(blink::mojom::FileSystemAccessErrorPtr)> callback, std::vector<scoped_refptr<FileSystemAccessLockManager::LockHandle>> locks, 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 ae79806..5ccf728f 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 @@ -43,6 +43,33 @@ namespace { +namespace { +// Helper to grant write permission to the parent directory of a target file. +void GrantParentDirectoryAccess( + content::FileSystemAccessPermissionContext* permission_context, + content::WebContents* web_contents, + const url::Origin& origin, + const base::FilePath& parent_dir) { + auto parent_grant = permission_context->GetWritePermissionGrant( + origin, content::PathInfo(parent_dir), + ChromeFileSystemAccessPermissionContext::HandleType::kDirectory, + ChromeFileSystemAccessPermissionContext::UserAction::kOpen); + base::test::TestFuture< + content::FileSystemAccessPermissionGrant::PermissionRequestOutcome> + parent_grant_future; + auto* rfh = web_contents->GetPrimaryMainFrame(); + parent_grant->RequestPermission( + content::GlobalRenderFrameHostId(rfh->GetProcess()->GetDeprecatedID(), + rfh->GetRoutingID()), + content::FileSystemAccessPermissionGrant::UserActivationState:: + kNotRequired, + parent_grant_future.GetCallback()); + ASSERT_EQ(parent_grant_future.Get(), + content::FileSystemAccessPermissionGrant::PermissionRequestOutcome:: + kUserGranted); +} +} // namespace + class TestFileSystemAccessPermissionContext : public ChromeFileSystemAccessPermissionContext { public: @@ -322,6 +349,10 @@ const base::FilePath test_file_path2 = CreateTestFile("test file contents 2"); SetUpAndGetHandleWithInitialPermissions("handle2", test_file_path2); + // Grant write permission to the parent directory. + GrantParentDirectoryAccess(permission_context(), GetWebContents(), + GetOrigin(), temp_dir().GetPath()); + // Rename the new file to the removed file path. ASSERT_TRUE(content::ExecJs( GetWebContents(), @@ -928,8 +959,7 @@ const url::Origin origin = GetOrigin(); // Grant the origin Extended Permission at the start of the test. - // Unlike `RestoreReadOnWrite_Rename` test case which doesn't use - // `showDirectoryPicker()`, this step is necessary in this test to prevent the + // This step is necessary in this test to prevent the // subsequent `showDirectoryPicker()` call from wiping all pre-existing // "dormant" grants from the two files handles when it reaches // `UpdateGrantsOnPermissionRequestResult()`. @@ -980,17 +1010,37 @@ const GURL url = embedded_test_server()->GetURL("/empty.html"); ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url)); + const url::Origin origin = GetOrigin(); + // Granting parent directory permission implicitly activates the origin's + // persistent permission state. Grant the origin Extended Permission at the + // start of the test in order to avoid changing the expected persistent + // permission state in mid-test. + permission_context()->SetOriginHasExtendedPermissionForTesting(origin); + // Create a file and get its handle. const base::FilePath test_file_path = CreateTestFile("test file contents"); - SetUpAndGetHandleWithInitialPermissions("handle", test_file_path); ... (file diff truncated)
Loading diff…
Regression Test / PoC
shipped with the fix
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 ae79806..5ccf728f 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
@@ -43,6 +43,33 @@
namespace {
+namespace {
+// Helper to grant write permission to the parent directory of a target file.
+void GrantParentDirectoryAccess(
+ content::FileSystemAccessPermissionContext* permission_context,
+ content::WebContents* web_contents,
+ const url::Origin& origin,
+ const base::FilePath& parent_dir) {
+ auto parent_grant = permission_context->GetWritePermissionGrant(
+ origin, content::PathInfo(parent_dir),
+ ChromeFileSystemAccessPermissionContext::HandleType::kDirectory,
+ ChromeFileSystemAccessPermissionContext::UserAction::kOpen);
+ base::test::TestFuture<
+ content::FileSystemAccessPermissionGrant::PermissionRequestOutcome>
+ parent_grant_future;
+ auto* rfh = web_contents->GetPrimaryMainFrame();
+ parent_grant->RequestPermission(
+ content::GlobalRenderFrameHostId(rfh->GetProcess()->GetDeprecatedID(),
+ rfh->GetRoutingID()),
+ content::FileSystemAccessPermissionGrant::UserActivationState::
+ kNotRequired,
+ parent_grant_future.GetCallback());
+ ASSERT_EQ(parent_grant_future.Get(),
+ content::FileSystemAccessPermissionGrant::PermissionRequestOutcome::
+ kUserGranted);
+}
+} // namespace
+
class TestFileSystemAccessPermissionContext
: public ChromeFileSystemAccessPermissionContext {
public:
@@ -322,6 +349,10 @@
const base::FilePath test_file_path2 = CreateTestFile("test file contents 2");
SetUpAndGetHandleWithInitialPermissions("handle2", test_file_path2);
+ // Grant write permission to the parent directory.
+ GrantParentDirectoryAccess(permission_context(), GetWebContents(),
+ GetOrigin(), temp_dir().GetPath());
+
// Rename the new file to the removed file path.
ASSERT_TRUE(content::ExecJs(
GetWebContents(),
@@ -928,8 +959,7 @@
const url::Origin origin = GetOrigin();
// Grant the origin Extended Permission at the start of the test.
- // Unlike `RestoreReadOnWrite_Rename` test case which doesn't use
- // `showDirectoryPicker()`, this step is necessary in this test to prevent the
+ // This step is necessary in this test to prevent the
// subsequent `showDirectoryPicker()` call from wiping all pre-existing
// "dormant" grants from the two files handles when it reaches
// `UpdateGrantsOnPermissionRequestResult()`.
@@ -980,17 +1010,37 @@
const GURL url = embedded_test_server()->GetURL("/empty.html");
ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url));
+ const url::Origin origin = GetOrigin();
+ // Granting parent directory permission implicitly activates the origin's
+ // persistent permission state. Grant the origin Extended Permission at the
+ // start of the test in order to avoid changing the expected persistent
+ // permission state in mid-test.
+ permission_context()->SetOriginHasExtendedPermissionForTesting(origin);
+
// Create a file and get its handle.
const base::FilePath test_file_path = CreateTestFile("test file contents");
- SetUpAndGetHandleWithInitialPermissions("handle", test_file_path);
+ SetUpAndGetHandleWithInitialPermissions("handle", test_file_path,
+ /*expect_extended_grants=*/true);
// Remove the file.
- const url::Origin origin = GetOrigin();
- RemoveFileAndVerifyPermissionsRevoked("handle", origin, test_file_path);
+ RemoveFileAndVerifyPermissionsRevoked("handle", origin, test_file_path,
+ /*expect_extended_write=*/true);
// Create a new file handle at a different path.
const base::FilePath test_file_path2 = CreateTestFile("test file contents 2");
- SetUpAndGetHandleWithInitialPermissions("handle2", test_file_path2);
+ SetUpAndGetHandleWithInitialPermissions("handle2", test_file_path2,
+ /*expect_extended_grants=*/true);
+
+ // Grant write permission to the parent directory.
+ GrantParentDirectoryAccess(permission_context(), GetWebContents(), origin,
+ temp_dir().GetPath());
+
+ // Wait for database write of handle2 to finish to avoid race condition
+ // between `ObjectPermissionContextBase::GrantObjectPermission()` called by
+ // `SetUpAndGetHandleWithInitialPermissions()` above and
+ // `ObjectPermissionContextBase::GetGrantedObject()` called by
+ // `handle2.move()` below.
+ content::RunAllTasksUntilIdle();
// Rename the new file to the removed file path.
ASSERT_TRUE(content::ExecJs(
@@ -1002,7 +1052,8 @@
// After renaming, read permission should be restored for the previously
// removed file path.
- VerifyPermissionsRestored(origin, test_file_path);
+ VerifyPermissionsRestored(origin, test_file_path,
+ /*expect_extended_grants=*/true);
ui::SelectFileDialog::SetFactory(nullptr);
}
@@ -1239,6 +1290,12 @@
// manual click.
permission_context.set_auto_abort_on_confirm_sensitive_entry_access();
+ // Grant write permission to the parent directory.
+ GrantParentDirectoryAccess(
+ &permission_context, GetWebContents(),
+ GetWebContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin(),
+ test_file.DirName());
+
EXPECT_THAT(
EvalJs(GetWebContents(),
R"(
@@ -1355,3 +1412,49 @@
EXPECT_NE(result, content::FileSystemAccessPermissionGrant::
PermissionRequestOutcome::kGrantedByRestorePrompt);
}
+
+class ChromeFileSystemAccessPermissionContextParentWriteRequiredBrowserTest
+ : public ChromeFileSystemAccessPermissionContextBrowserTestBase {
+ public:
+ ChromeFileSystemAccessPermissionContextParentWriteRequiredBrowserTest() {
+ // On some bots, a temporary directory and the home directory are the same,
+ // and on others they are not. Enable the feature and set `OnlyInHomedir` to
+ // false so the test always blocks renaming.
+ scoped_feature_list_.InitFromCommandLine(
+ "FileSystemAccessRenameRequiresParentWritePermission:OnlyInHomedir/"
+ "false",
+ "");
+ }
+
+ private:
+ base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+// Verify that renaming a local file fails with NotAllowedError when the website
+// does not have write access to the parent directory.
+IN_PROC_BROWSER_TEST_F(
+ ChromeFileSystemAccessPermissionContextParentWriteRequiredBrowserTest,
+ Rename_NoParentWriteAccessFails) {
+ // Navigate to a test page.
+ const GURL url = embedded_test_server()->GetURL("/empty.html");
+ ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url));
+ // Create a file and get its handle.
+ const base::FilePath test_file_path = CreateTestFile("test file contents");
+ SetUpAndGetHandleWithInitialPermissions("handle", test_file_path);
+ // We do not grant write permission to the parent directory.
+ // Rename the file. It should fail with NotAllowedError.
+ EXPECT_EQ(
+ "NotAllowedError",
+ content::EvalJs(GetWebContents(), content::JsReplace(R"((async () => {
+ try {
+ await self.handle.move($1);
+ return "success";
+ } catch (e) {
+ return e.name;
+ }
+ })())",
+ "new_name.txt")));
+ // Verify the file was not renamed and old file still exists on disk.
+ base::ScopedAllowBlockingForTesting allow_blocking;
+ EXPECT_TRUE(base::PathExists(test_file_path));
+}
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 59d51b1c..f24e450 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
@@ -11,6 +11,7 @@
#include <utility>
#include <vector>
+#include "base/base_paths.h"
#include "base/command_line.h"
#include "base/feature_list.h"
#include "base/files/file_path.h"
@@ -24,6 +25,7 @@
#include "base/test/gmock_callback_support.h"
#include "base/test/gmock_expected_support.h"
#include "base/test/scoped_feature_list.h"
+#include "base/test/scoped_path_override.h"
#include "base/test/task_environment.h"
#include "base/test/test_future.h"
#include "build/build_config.h"
@@ -1104,6 +1106,8 @@
bool target_present;
// Does the site have user activation?
bool gesture_present;
+ // Is the parent directory check feature enabled?
+ bool parent_check_enabled;
};
// Uses a mock permission context to ensure the correct permission grant for the
@@ -1122,7 +1126,16 @@
void SetUp() override {
base::CommandLine::ForCurrentProcess()->AppendSwitch(
switches::kEnableExperimentalWebPlatformFeatures);
-
+ if (GetParam().parent_check_enabled) {
+ // Enable the feature and FORCE it to run everywhere (`only_in_homedir`
+ // = false) so we can test it in the test's temporary directory.
+ scoped_feature_list.InitAndEnableFeatureWithParameters(
+ features::kFileSystemAccessRenameRequiresParentWritePermission,
+ {{"OnlyInHomedir", "false"}});
+ } else {
+ scoped_feature_list.InitAndDisableFeature(
+ features::kFileSystemAccessRenameRequiresParentWritePermission);
+ }
SetupHelper(storage::kFileSystemTypeLocal, /*is_incognito=*/false,
/*use_content_uri=*/false);
manager_->SetPermissionContextForTesting(&permission_context_);
@@ -1145,22 +1158,12 @@
return {source, target};
}
- void ExpectFileRenameSuccess(
- const base::FilePath& source,
+ void SetupRenamePermissionExpectations(
+ const url::Origin& origin,
const base::FilePath& target,
- scoped_refptr<FixedFileSystemAccessPermissionGrant> target_grant) {
- auto source_handle =
- GetHandleWithPermissions(source, /*read_grant=*/allow_grant_,
- /*write_grant=*/allow_grant_);
- auto target_handle =
- GetHandleWithPermissions(target, /*read_grant=*/target_grant,
- /*write_grant=*/target_grant);
- auto origin = test_src_storage_key_.origin();
- auto target_basename = target.BaseName();
-
- EXPECT_CALL(permission_context_,
- IsFileTypeDangerous_(target_basename, origin))
- .WillOnce(testing::Return(false));
+ scoped_refptr<FixedFileSystemAccessPermissionGrant> target_grant,
+ bool parent_check_enabled,
+ scoped_refptr<FixedFileSystemAccessPermissionGrant> parent_grant) {
EXPECT_CALL(permission_context_,
GetReadPermissionGrant(
origin, content::PathInfo(target),
@@ -1173,58 +1176,37 @@
FileSystemAccessPermissionContext::HandleType::kFile,
FileSystemAccessPermissionContext::UserAction::kNone))
.WillOnce(testing::Return(target_grant));
- EXPECT_CALL(
- permission_context_,
- ConfirmSensitiveEntryAccess_(
- origin, content::PathInfo(target),
- FileSystemAccessPermissionContext::HandleType::kFile,
- FileSystemAccessPermissionContext::UserAction::kSave,
- web_contents_->GetPrimaryMainFrame()->GetGlobalId(), testing::_))
- .WillOnce(base::test::RunOnceCallback<5>(
- FileSystemAccessPermissionContext::SensitiveEntryResult::kAllowed));
- // These checks should only be called if the file is successfully moved.
+ const bool target_write_granted =
+ target_grant->GetStatus() == PermissionStatus::GRANTED;
- if (source != target) {
- // On Windows, CreateTemporaryFileInDir() creates files with the '.tmp'
- // extension. Safe Browsing checks are not run on same-file-system moves
- // in which the extension does not change. For more context, see
- // FileSystemAccessSafeMoveHelper::RequireAfterWriteChecks().
- if (source.Extension() != FILE_PATH_LITERAL(".tmp") ||
- target.Extension() != FILE_PATH_LITERAL(".tmp")) {
- EXPECT_CALL(
- permission_context_,
- PerformAfterWriteChecks_(testing::_, testing::_, testing::_))
- .WillOnce(base::test::RunOnceCallback<2>(
- FileSystemAccessPermissionContext::AfterWriteCheckResult::
- kAllow));
- }
+ if (parent_check_enabled && !target_write_granted) {
+ auto parent_path = target.DirName();
+ EXPECT_CALL(permission_context_,
+ GetReadPermissionGrant(
+ origin, content::PathInfo(parent_path),
+ FileSystemAccessPermissionContext::HandleType::kDirectory,
+ FileSystemAccessPermissionContext::UserAction::kNone))
+ .WillOnce(testing::Return(parent_grant));
+ EXPECT_CALL(permission_context_,
+ GetWritePermissionGrant(
+ origin, content::PathInfo(parent_path),
+ FileSystemAccessPermissionContext::HandleType::kDirectory,
+ FileSystemAccessPermissionContext::UserAction::kNone))
+ .WillOnce(testing::Return(parent_grant));
}
- EXPECT_CALL(permission_context_,
- NotifyEntryMoved(origin, PathInfo(source), PathInfo(target)));
-
- if (gesture_present()) {
... (truncated)
Loading diff…
Original Bug Report
reported by [email protected]
Sandbox escape to RCE via File System Access API move() to shell dotfiles
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 without the security team.
Overview: The File System Access API allows a site with write access to a file in the user’s home directory to rename it to a shell initialization dotfile like .zshenv. This bypasses security checks because leading dots are stripped during path validation, and files in the home directory are generally allowed. By tricking a user into saving a file to their home directory, an attacker can rename it to a dotfile and achieve persistent RCE when the user opens a terminal.
Affected files:
content/browser/file_system_access/file_system_access_handle_base.ccchrome/browser/file_system_access/chrome_file_system_access_permission_context.cccontent/browser/file_system_access/file_system_access_manager_impl.ccthird_party/blink/renderer/modules/file_system_access/file_system_file_handle.ccthird_party/blink/renderer/modules/file_system_access/file_system_file_handle.idl
Estimated timestamp from git blame: 2025-07-03
Vulnerability Details
The File System Access API’s move() method allows a website to rename a file to a sibling name within the same directory. If a user is socially engineered into saving a file in their home directory (e.g., ~/innocent.txt), the attacker obtains a FileSystemFileHandle with explicit write access to that file.
The attacker can then write a malicious shell payload to the file and wait for a transient user activation (such as a click). Upon activation, the site can call handle.move(".zshenv") (on macOS) or handle.move(".bash_aliases") (on Linux).
This rename operation systematically bypasses several critical security checks in the browser process:
- User Activation Gate Bypass:
FileSystemAccessHandleBase::DoRenamepermits the move without explicit write access to the destination ifhas_transient_user_activationis true. - Safe Path Component Check:
FileSystemAccessManagerImpl::IsSafePathComponentstrips leading dots before checking filename legality. It treats the entire filename (e.g.,.zshenv) as the extension, strips the dot, and checkszshenvagainst Safe Browsing policies. Sincezshenvis not explicitly listed as dangerous indownload_file_types.asciipb, it defaults toNOT_DANGEROUSand passes the check. - Blocklist Check:
ChromeFileSystemAccessPermissionContext::GenerateBlockPathsexplicitly marks the home directory (base::DIR_HOME) withBlockType::kDontBlockChildren. While sensitive subdirectories like~/.sshand~/.configare blocked, individual sibling files in the home directory are implicitly allowed. - Overwrite Check: As long as the destination file (e.g.,
~/.zshenv) does not already exist, the move proceeds silently without prompting the user.
Once the file is renamed, the next time the user opens a terminal, their shell (e.g., zsh on macOS) will source the malicious dotfile, resulting in Remote Code Execution outside the Chrome sandbox.
Potential Reproduction Steps
Note: These are suggested steps based on code analysis; a working proof of concept has not yet been executed by our tooling.
- An attacker sets up a malicious website and tricks the user into saving a file using
window.showSaveFilePicker(). The user is instructed to save the file in their home directory (~) asinnocent.txt. - The website writes a shell command payload (e.g.,
touch /tmp/rce) to the file viacreateWritable(). - The site waits for the user to interact with the page again (e.g., a “Complete Download” button) to gain transient user activation.
- The site calls
handle.move(".zshenv")(macOS) orhandle.move(".bash_aliases")(Linux). - The browser renames the file on disk to the target shell initialization file.
- The user opens a new terminal session, which automatically sources the malicious file and executes the payload.
Suggested Fix
- Block Shell Dotfiles: Update
ChromeFileSystemAccessPermissionContext::GenerateBlockPathsto explicitly block known shell initialization files (e.g.,.bashrc,.bash_profile,.zshenv,.zshrc,.profile,.bash_aliases) in the home directory, similar to how.sshand.gnupgare blocked. - Update Safe Browsing Policies: Add these shell configuration file extensions to
download_file_types.asciipbasDANGEROUSto ensure they are caught byIsFileTypeDangerousduring rename operations. - Re-evaluate Dotfile Handling: Reconsider whether
FileSystemAccessManagerImpl::IsSafePathComponentshould allow renaming to arbitrary dotfiles, or if it should more strictly restrict which dotfiles can be created via the API.
Evaluated with Chrome root at commit: ff3d2b74fa39431785bd60e51463b08fcc71ee33
Results so far have been promising, but there can be wrong deductions. If this proves to be a false positive, please close as WAI; 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.
References
On This Page