mirrors/electron
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions 1

fix: two possible FSA crashes (#45260)

Browse source 
* 5786874: Change Observer: Fix crash when navigating to new page

5786874

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

* 5794141: Change Observer: Fix Get*PermissionGrant crash

5794141

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
This commit is contained in:
trop[bot] 2025-01-20 10:36:09 -06:00 • committed by GitHub
parent 41c1161dd9
commit 2c22507dfb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
1 changed files with 34 additions and 18 deletions
Download patch file Download diff file Expand all files Collapse all files

52
shell/browser/file_system_access/file_system_access_permission_context.cc
View file

@ -459,7 +459,7 @@ FileSystemAccessPermissionContext::GetReadPermissionGrant(
// but that is exactly what we want. // but that is exactly what we want.
auto& origin_state = active_permissions_map_[origin]; auto& origin_state = active_permissions_map_[origin];
auto*& existing_grant = origin_state.read_grants[path_info.path]; auto*& existing_grant = origin_state.read_grants[path_info.path];
scoped_refptr<PermissionGrantImpl> new_grant; scoped_refptr<PermissionGrantImpl> grant;
if (existing_grant && existing_grant->handle_type() != handle_type) { if (existing_grant && existing_grant->handle_type() != handle_type) {
// |path| changed from being a directory to being a file or vice versa, // |path| changed from being a directory to being a file or vice versa,
@ -469,18 +469,21 @@ FileSystemAccessPermissionContext::GetReadPermissionGrant(
existing_grant = nullptr; existing_grant = nullptr;
} }
if (!existing_grant) { bool creating_new_grant = !existing_grant;
new_grant = base::MakeRefCounted<PermissionGrantImpl>( if (creating_new_grant) {
grant = base::MakeRefCounted<PermissionGrantImpl>(
weak_factory_.GetWeakPtr(), origin, path_info, handle_type, weak_factory_.GetWeakPtr(), origin, path_info, handle_type,
GrantType::kRead, user_action); GrantType::kRead, user_action);
existing_grant = new_grant.get(); existing_grant = grant.get();
} else {
grant = existing_grant;
} }
// If a parent directory is already readable this new grant should also be // If a parent directory is already readable this new grant should also be
// readable. // readable.
if (new_grant && if (creating_new_grant &&
AncestorHasActivePermission(origin, path_info.path, GrantType::kRead)) { AncestorHasActivePermission(origin, path_info.path, GrantType::kRead)) {
existing_grant->SetStatus(PermissionStatus::GRANTED); grant->SetStatus(PermissionStatus::GRANTED);
} else { } else {
switch (user_action) { switch (user_action) {
case UserAction::kOpen: case UserAction::kOpen:
@ -492,7 +495,7 @@ FileSystemAccessPermissionContext::GetReadPermissionGrant(
[[fallthrough]]; [[fallthrough]];
case UserAction::kDragAndDrop: case UserAction::kDragAndDrop:
// Drag&drop grants read access for all handles. // Drag&drop grants read access for all handles.
existing_grant->SetStatus(PermissionStatus::GRANTED); grant->SetStatus(PermissionStatus::GRANTED);
break; break;
case UserAction::kLoadFromStorage: case UserAction::kLoadFromStorage:
case UserAction::kNone: case UserAction::kNone:
@ -500,7 +503,7 @@ FileSystemAccessPermissionContext::GetReadPermissionGrant(
} }
} }
return existing_grant; return grant;
} }
scoped_refptr<content::FileSystemAccessPermissionGrant> scoped_refptr<content::FileSystemAccessPermissionGrant>
@ -514,7 +517,7 @@ FileSystemAccessPermissionContext::GetWritePermissionGrant(
// but that is exactly what we want. // but that is exactly what we want.
auto& origin_state = active_permissions_map_[origin]; auto& origin_state = active_permissions_map_[origin];
auto*& existing_grant = origin_state.write_grants[path_info.path]; auto*& existing_grant = origin_state.write_grants[path_info.path];
scoped_refptr<PermissionGrantImpl> new_grant; scoped_refptr<PermissionGrantImpl> grant;
if (existing_grant && existing_grant->handle_type() != handle_type) { if (existing_grant && existing_grant->handle_type() != handle_type) {
// |path| changed from being a directory to being a file or vice versa, // |path| changed from being a directory to being a file or vice versa,
@ -524,23 +527,26 @@ FileSystemAccessPermissionContext::GetWritePermissionGrant(
existing_grant = nullptr; existing_grant = nullptr;
} }
if (!existing_grant) { bool creating_new_grant = !existing_grant;
new_grant = base::MakeRefCounted<PermissionGrantImpl>( if (creating_new_grant) {
grant = base::MakeRefCounted<PermissionGrantImpl>(
weak_factory_.GetWeakPtr(), origin, path_info, handle_type, weak_factory_.GetWeakPtr(), origin, path_info, handle_type,
GrantType::kWrite, user_action); GrantType::kWrite, user_action);
existing_grant = new_grant.get(); existing_grant = grant.get();
} else {
grant = existing_grant;
} }
// If a parent directory is already writable this new grant should also be // If a parent directory is already writable this new grant should also be
// writable. // writable.
if (new_grant && if (creating_new_grant &&
AncestorHasActivePermission(origin, path_info.path, GrantType::kWrite)) { AncestorHasActivePermission(origin, path_info.path, GrantType::kWrite)) {
existing_grant->SetStatus(PermissionStatus::GRANTED); grant->SetStatus(PermissionStatus::GRANTED);
} else { } else {
switch (user_action) { switch (user_action) {
case UserAction::kSave: case UserAction::kSave:
// Only automatically grant write access for save dialogs. // Only automatically grant write access for save dialogs.
existing_grant->SetStatus(PermissionStatus::GRANTED); grant->SetStatus(PermissionStatus::GRANTED);
break; break;
case UserAction::kOpen: case UserAction::kOpen:
case UserAction::kDragAndDrop: case UserAction::kDragAndDrop:
@ -550,7 +556,7 @@ FileSystemAccessPermissionContext::GetWritePermissionGrant(
} }
} }
return existing_grant; return grant;
} }
bool FileSystemAccessPermissionContext::IsFileTypeDangerous( bool FileSystemAccessPermissionContext::IsFileTypeDangerous(
@ -864,12 +870,22 @@ void FileSystemAccessPermissionContext::RevokeActiveGrants(
auto origin_it = active_permissions_map_.find(origin); auto origin_it = active_permissions_map_.find(origin);
if (origin_it != active_permissions_map_.end()) { if (origin_it != active_permissions_map_.end()) {
OriginState& origin_state = origin_it->second; OriginState& origin_state = origin_it->second;
for (auto& grant : origin_state.read_grants) { for (auto grant_iter = origin_state.read_grants.begin(),
grant_end = origin_state.read_grants.end();
grant_iter != grant_end;) {
// The grant may be removed from `read_grants`, so increase the iterator
// before continuing.
auto& grant = *(grant_iter++);
if (file_path.empty() || grant.first == file_path) { if (file_path.empty() || grant.first == file_path) {
grant.second->SetStatus(PermissionStatus::ASK); grant.second->SetStatus(PermissionStatus::ASK);
} }
} }
for (auto& grant : origin_state.write_grants) { for (auto grant_iter = origin_state.write_grants.begin(),
grant_end = origin_state.write_grants.end();
grant_iter != grant_end;) {
// The grant may be removed from `write_grants`, so increase the iterator
// before continuing.
auto& grant = *(grant_iter++);
if (file_path.empty() || grant.first == file_path) { if (file_path.empty() || grant.first == file_path) {
grant.second->SetStatus(PermissionStatus::ASK); grant.second->SetStatus(PermissionStatus::ASK);
} }