From 681a2c1aba825f9fe4c4324cd27f910794051e21 Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Wed, 4 Feb 2026 16:12:59 +0100 Subject: [PATCH] fix: possible crash in FileSystem API (#49634) Refs https://chromium-review.googlesource.com/6880247 Fixes a crash that can arise in the File System Access API in the following scenario: 1. Create fileHandle1 at path1. 2. Call fileHandle1.remove() or user manually delete the file. 3. Create fileHandle2 at path2. 4. fileHandle2.move(path1). Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr --- .../file_system_access_permission_context.cc | 47 ++++++++++++++----- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/shell/browser/file_system_access/file_system_access_permission_context.cc b/shell/browser/file_system_access/file_system_access_permission_context.cc index 2dcbfcb95c..0ae068d221 100644 --- a/shell/browser/file_system_access/file_system_access_permission_context.cc +++ b/shell/browser/file_system_access/file_system_access_permission_context.cc @@ -400,31 +400,47 @@ class FileSystemAccessPermissionContext::PermissionGrantImpl } } + // Updates the in-memory permission grant for the `new_path` in the `grants` + // map using the same grant from the `old_path`, and removes the grant entry + // for the `old_path`. + // If `allow_overwrite` is true, this will replace any pre-existing grant at + // `new_path`. static void UpdateGrantPath( std::map& grants, const content::PathInfo& old_path, - const content::PathInfo& new_path) { + const content::PathInfo& new_path, + bool allow_overwrite) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); - auto entry_it = + auto old_path_it = std::ranges::find_if(grants, [&old_path](const auto& entry) { return entry.first == old_path.path; }); - if (entry_it == grants.end()) { - // There must be an entry for an ancestor of this entry. Nothing to do - // here. + if (old_path_it == grants.end()) { return; } - DCHECK_EQ(entry_it->second->GetActivePermissionStatus(), + DCHECK_EQ(old_path_it->second->GetActivePermissionStatus(), PermissionStatus::GRANTED); - auto* const grant_impl = entry_it->second; - grant_impl->SetPath(new_path); + auto* const grant_to_move = old_path_it->second; - // Update the permission grant's key in the map of active permissions. - grants.erase(entry_it); - grants.emplace(new_path.path, grant_impl); + // See https://chromium-review.googlesource.com/4803165 + if (allow_overwrite) { + auto new_path_it = grants.find(new_path.path); + if (new_path_it != grants.end() && new_path_it->second != grant_to_move) { + new_path_it->second->SetStatus(PermissionStatus::DENIED); + } + } + + grant_to_move->SetPath(new_path); + + grants.erase(old_path_it); + if (allow_overwrite) { + grants.insert_or_assign(new_path.path, grant_to_move); + } else { + grants.emplace(new_path.path, grant_to_move); + } } protected: @@ -930,12 +946,17 @@ void FileSystemAccessPermissionContext::NotifyEntryMoved( return; } + // It's possible `new_path` already has existing persistent permission. + // See crbug.com/423663220. + bool allow_overwrite = base::FeatureList::IsEnabled( + features::kFileSystemAccessMoveWithOverwrite); + auto it = active_permissions_map_.find(origin); if (it != active_permissions_map_.end()) { PermissionGrantImpl::UpdateGrantPath(it->second.write_grants, old_path, - new_path); + new_path, allow_overwrite); PermissionGrantImpl::UpdateGrantPath(it->second.read_grants, old_path, - new_path); + new_path, allow_overwrite); } }