PatchGap
Chrome · Chrome for iOS

CVE-2026-10952

UAF in Chrome for iOS
High chrome UAF 📄 Reporter bug report 🔧 Commit mapped

Overview

High
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactUse after free in Chrome for iOS
DescriptionUse after free in Chrome for iOS
ComponentChrome for iOS
Bug ClassUAF
Tracker505231370
Fix commitfa48de327329 (chromium/src) +211/-102
CISA KEVNot listed
CreditedGoogle
Disclosed2026-06-02

Changed Functions

FunctionChangeNotes
if
ios/chrome/browser/bookmarks/folder_chooser/coordinator/bookmarks_folder_chooser_coordinator.mm
 modified
for
ios/chrome/browser/bookmarks/folder_chooser/coordinator/bookmarks_folder_chooser_coordinator.mm
 modified

Files Changed

  • ios/chrome/browser/bookmarks/folder_chooser/coordinator/bookmarks_folder_chooser_coordinator.h
  • ios/chrome/browser/bookmarks/folder_chooser/coordinator/bookmarks_folder_chooser_coordinator.mm
  • ios/chrome/browser/bookmarks/folder_chooser/coordinator/bookmarks_folder_chooser_mediator.h
  • ios/chrome/browser/bookmarks/folder_chooser/coordinator/bookmarks_folder_chooser_mediator.mm

Patch

view commit
From fa48de327329311bc10c386956ff5110ddd7d415 Mon Sep 17 00:00:00 2001
From: Arthur Milchior <arthurmilchior@google.com>
Date: Wed, 03 Jun 2026 08:03:01 -0700
Subject: [PATCH] [ios][bookmarks] Use node IDs instead of raw_ptr in folder chooser

Replaces std::set<raw_ptr<const BookmarkNode>> and std::vector<raw_ptr<const BookmarkNode>> in BookmarksFolderChooserMediator, BookmarksFolderChooserViewController, and BookmarksFolderChooserSubDataSource with std::set<int64_t> and std::vector<int64_t> respectively.

Looks up nodes dynamically using bookmarks::GetBookmarkNodeByID, handles deleted nodes gracefully, and exits if the container is empty due to all nodes being deleted.

Bug: 505231370
Change-Id: Ib031b0d97393ec8a0057d87a063b3b5e704f31ae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7879954
Reviewed-by: Huiting Yu <huitingyu@google.com>
Commit-Queue: Arthur Milchior <arthurmilchior@chromium.org>
Auto-Submit: Arthur Milchior <arthurmilchior@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1640946}
---

diff --git a/ios/chrome/browser/bookmarks/folder_chooser/coordinator/bookmarks_folder_chooser_coordinator.h b/ios/chrome/browser/bookmarks/folder_chooser/coordinator/bookmarks_folder_chooser_coordinator.h
index fca4937..04ab47fd 100644
--- a/ios/chrome/browser/bookmarks/folder_chooser/coordinator/bookmarks_folder_chooser_coordinator.h
+++ b/ios/chrome/browser/bookmarks/folder_chooser/coordinator/bookmarks_folder_chooser_coordinator.h
@@ -66,7 +66,7 @@
 - (BOOL)canDismiss;
 // The value of `movedNodes` during init, minus the nodes that have been
 // deleted in the meantime.
-- (const std::set<raw_ptr<const bookmarks::BookmarkNode>>&)movedNodes;
+- (std::set<raw_ptr<const bookmarks::BookmarkNode>>)movedNodes;
 // Puts a blue check mark beside a folder it in the UI.
 // If unset no blue check mark is shown.
 - (void)setSelectedFolder:(const bookmarks::BookmarkNode*)folder;
diff --git a/ios/chrome/browser/bookmarks/folder_chooser/coordinator/bookmarks_folder_chooser_coordinator.mm b/ios/chrome/browser/bookmarks/folder_chooser/coordinator/bookmarks_folder_chooser_coordinator.mm
index 183efd5..5a741523 100644
--- a/ios/chrome/browser/bookmarks/folder_chooser/coordinator/bookmarks_folder_chooser_coordinator.mm
+++ b/ios/chrome/browser/bookmarks/folder_chooser/coordinator/bookmarks_folder_chooser_coordinator.mm
@@ -47,9 +47,9 @@
   BookmarksFolderChooserViewController* _viewController;
   // Coordinator to show the folder editor UI.
   BookmarksFolderEditorCoordinator* _folderEditorCoordinator;
-  // List of nodes to hide when displaying folders. This is to avoid to move a
-  // folder inside a child folder.
-  std::set<raw_ptr<const bookmarks::BookmarkNode>> _movedNodes;
+  // List of id of moved nodes. This is to avoid to move a
+  // folder inside a child folder. Only set between init and start.
+  std::set<int64_t> _movedNodeIds;
   // The folder that has a blue check mark beside it in the UI.
   // This is only used for clients of this coordinator to update the UI. This
   // does not reflect the folder users chose by clicking. For that information
@@ -84,7 +84,9 @@
                                    movedNodes {
   self = [super initWithBaseViewController:viewController browser:browser];
   if (self) {
-    _movedNodes = movedNodes;
+    for (const raw_ptr<const bookmarks::BookmarkNode>& node : movedNodes) {
+      _movedNodeIds.insert(node->id());
+    }
     _allowsNewFolders = YES;
   }
   return self;
@@ -97,7 +99,7 @@
   return YES;
 }
 
-- (const std::set<raw_ptr<const bookmarks::BookmarkNode>>&)movedNodes {
+- (std::set<raw_ptr<const bookmarks::BookmarkNode>>)movedNodes {
   return [_mediator movedNodes];
 }
 
@@ -127,10 +129,10 @@
   syncer::SyncService* syncService = SyncServiceFactory::GetForProfile(profile);
   _mediator = [[BookmarksFolderChooserMediator alloc]
       initWithBookmarkModel:model
-                 movedNodes:std::move(_movedNodes)
+               movedNodeIds:std::move(_movedNodeIds)
       authenticationService:authenticationService
                 syncService:syncService];
-  _movedNodes.clear();
+  _movedNodeIds.clear();
   _mediator.delegate = self;
   _mediator.selectedFolderNode = _selectedFolder;
   _viewController = [[BookmarksFolderChooserViewController alloc]
diff --git a/ios/chrome/browser/bookmarks/folder_chooser/coordinator/bookmarks_folder_chooser_mediator.h b/ios/chrome/browser/bookmarks/folder_chooser/coordinator/bookmarks_folder_chooser_mediator.h
index e82fe56..18d00c11b 100644
--- a/ios/chrome/browser/bookmarks/folder_chooser/coordinator/bookmarks_folder_chooser_mediator.h
+++ b/ios/chrome/browser/bookmarks/folder_chooser/coordinator/bookmarks_folder_chooser_mediator.h
@@ -36,22 +36,23 @@
 
 // Initialize the mediator with a bookmark model.
 // `model` must not be `nullptr` and must be loaded.
-// `movedNodes` are the list of nodes to hide when displaying folders. This is
-// to avoid to move a folder inside a child folder. These are also the list of
-// nodes that are being moved to a folder.
-- (instancetype)
-    initWithBookmarkModel:(bookmarks::BookmarkModel*)model
-               movedNodes:
-                   (std::set<raw_ptr<const bookmarks::BookmarkNode>>)movedNodes
-    authenticationService:(AuthenticationService*)authenticationService
-              syncService:(syncer::SyncService*)syncService
+// `movedNodeIds` are the list of nodes to hide when displaying folders. This
+// is to avoid to move a folder inside a child folder. These are also the list
+// of nodes that are being moved to a folder.
+- (instancetype)initWithBookmarkModel:(bookmarks::BookmarkModel*)model
+                         movedNodeIds:(std::set<int64_t>)movedNodeIds
+                authenticationService:
+                    (AuthenticationService*)authenticationService
+                          syncService:(syncer::SyncService*)syncService
     NS_DESIGNATED_INITIALIZER;
 
 - (instancetype)init NS_UNAVAILABLE;
 
 - (void)disconnect;
 
-- (const std::set<raw_ptr<const bookmarks::BookmarkNode>>&)movedNodes;
+// Returns the set of nodes that were selected when the view was opened and has
+// not been deleted in the meantime.
+- (std::set<raw_ptr<const bookmarks::BookmarkNode>>)movedNodes;
 
 @end
 
diff --git a/ios/chrome/browser/bookmarks/folder_chooser/coordinator/bookmarks_folder_chooser_mediator.mm b/ios/chrome/browser/bookmarks/folder_chooser/coordinator/bookmarks_folder_chooser_mediator.mm
index fb0f8d8..1145352 100644
--- a/ios/chrome/browser/bookmarks/folder_chooser/coordinator/bookmarks_folder_chooser_mediator.mm
+++ b/ios/chrome/browser/bookmarks/folder_chooser/coordinator/bookmarks_folder_chooser_mediator.mm
@@ -36,8 +36,8 @@
   BookmarksFolderChooserSubDataSourceImpl* _accountDataSource;
   // Set of nodes to hide when displaying folders. This is to avoid to move a
   // folder inside a child folder. These are also the list of nodes that are
-  // being moved to a folder.
-  std::set<raw_ptr<const BookmarkNode>> _movedNodes;
+  // being moved (moved to a folder).
+  std::set<int64_t> _movedNodeIds;
   // Observer for signin status changes.
   std::unique_ptr<AuthenticationServiceObserverBridge> _authServiceBridge;
   // Sync service.
@@ -51,8 +51,7 @@
 @synthesize UIDisabled = _UIDisabled;
 
 - (instancetype)initWithBookmarkModel:(bookmarks::BookmarkModel*)model
-                           movedNodes:
-                               (std::set<raw_ptr<const BookmarkNode>>)movedNodes
+                         movedNodeIds:(std::set<int64_t>)movedNodeIds
                 authenticationService:(AuthenticationService*)authService
                           syncService:(syncer::SyncService*)syncService {
   CHECK(model, base::NotFatalUntil::M145);
@@ -72,7 +71,7 @@
                          type:BookmarkStorageType::kAccount
              parentDataSource:self];
 
-    _movedNodes = std::move(movedNodes);
+    _movedNodeIds = std::move(movedNodeIds);
     _authServiceBridge = std::make_unique<AuthenticationServiceObserverBridge>(
         authService, self);
     _syncService = syncService;
@@ -88,7 +87,7 @@
   [_accountDataSource disconnect];
   _accountDataSource.consumer = nil;
   _accountDataSource = nil;
-  _movedNodes.clear();
+  _movedNodeIds.clear();
   _authServiceBridge.reset();
   _syncService = nullptr;
   _syncObserverBridge = nullptr;
@@ -99,8 +98,9 @@
   DUMP_WILL_BE_CHECK(!_authServiceBridge);
 }
 
-- (const std::set<raw_ptr<const bookmarks::BookmarkNode>>&)movedNodes {
-  return _movedNodes;
+- (std::set<raw_ptr<const bookmarks::BookmarkNode>>)movedNodes {
+  return bookmark_utils_ios::GetBookmarkNodesByIds(_bookmarkModel,
+                                                   _movedNodeIds);
 }
 
 - (const bookmarks::BookmarkNode*)selectedFolderNode {
@@ -136,16 +136,17 @@
 #pragma mark - BookmarksFolderChooserParentDataSource
 
 - (void)bookmarkNodeDeleted:(const BookmarkNode*)bookmarkNode {
-  // Remove node from `_movedNodes` if it is already deleted (possibly remotely
-  // by another sync device).
-  if (_movedNodes.contains(bookmarkNode)) {
-    _movedNodes.erase(bookmarkNode);
-    // if `_movedNodes` becomes empty, nothing to move.  Exit the folder
+  // Remove node from `_movedNodeIds` if it is already deleted (possibly
+  // remotely by another sync device).
+  int64_t nodeId = bookmarkNode->id();
+  if (_movedNodeIds.contains(nodeId)) {
+    _movedNodeIds.erase(nodeId);
+    // if `_movedNodeIds` becomes empty, nothing to move.  Exit the folder
     // chooser.
-    if (_movedNodes.empty()) {
+    if (_movedNodeIds.empty()) {
       [_delegate bookmarksFolderChooserMediatorWantsDismissal:self];
     }
-    // Exit here because no visible node was deleted. Nodes in `_movedNodes`
+    // Exit here because no visible node was deleted. Nodes in `_movedNodeIds`
     // cannot be any visible folder in folder chooser.
     return;
   }
@@ -161,7 +162,7 @@
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/ios/chrome/browser/bookmarks/folder_chooser/test/bookmarks_folder_chooser_mediator_unittest.mm b/ios/chrome/browser/bookmarks/folder_chooser/test/bookmarks_folder_chooser_mediator_unittest.mm
index 6ffd11e..824eb3e 100644
--- a/ios/chrome/browser/bookmarks/folder_chooser/test/bookmarks_folder_chooser_mediator_unittest.mm
+++ b/ios/chrome/browser/bookmarks/folder_chooser/test/bookmarks_folder_chooser_mediator_unittest.mm
@@ -45,7 +45,7 @@
 
     mediator_ = [[BookmarksFolderChooserMediator alloc]
         initWithBookmarkModel:bookmark_model_
-                   movedNodes:{}
+                 movedNodeIds:{}
         authenticationService:authentication_service_
                   syncService:&sync_service_];
   }
diff --git a/ios/chrome/browser/bookmarks/folder_chooser/test/bookmarks_folder_chooser_sub_data_source_impl_unittest.mm b/ios/chrome/browser/bookmarks/folder_chooser/test/bookmarks_folder_chooser_sub_data_source_impl_unittest.mm
index e9814c1..23831ae 100644
--- a/ios/chrome/browser/bookmarks/folder_chooser/test/bookmarks_folder_chooser_sub_data_source_impl_unittest.mm
+++ b/ios/chrome/browser/bookmarks/folder_chooser/test/bookmarks_folder_chooser_sub_data_source_impl_unittest.mm
@@ -12,6 +12,7 @@
 #import "base/strings/sys_string_conversions.h"
 #import "components/bookmarks/browser/bookmark_model.h"
 #import "components/bookmarks/browser/bookmark_node.h"
+#import "components/bookmarks/browser/bookmark_utils.h"
 #import "components/bookmarks/common/bookmark_features.h"
 #import "ios/chrome/browser/bookmarks/folder_chooser/ui/bookmarks_folder_chooser_consumer.h"
 #import "ios/chrome/browser/bookmarks/model/bookmark_ios_unit_test_support.h"
@@ -55,7 +56,7 @@
   _movedNodes.clear();
 }
 
-- (const std::set<const BookmarkNode*>&)movedNodes {
+- (const std::set<const BookmarkNode*>)movedNodes {
   return _movedNodes;
 }
 
@@ -135,6 +136,13 @@
     bookmark_model_->Move(node, new_parent, new_parent->children().size());
   }
 
+  const BookmarkNode* GetNodeByID(int64_t nodeId) {
+    const BookmarkNode* node =
+        bookmarks::GetBookmarkNodeByID(bookmark_model_, nodeId);
+    EXPECT_NE(node, nullptr);
+    return node;
+  }
+
   BookmarksFolderChooserSubDataSourceImpl* sub_data_source_;
   id mock_consumer_;
   FakeBookmarksFolderChooserParentDataSource* fake_parent_data_source_;
@@ -144,7 +152,7 @@
 };
 
 // Tests that the sub data source correctly fetches visible folders.
-TEST_P(BookmarksFolderChooserSubDataSourceImplTest, TestVisibleFolderNodes) {
+TEST_P(BookmarksFolderChooserSubDataSourceImplTest, TestvisibleFolderNodeIds) {
   const BookmarkNode* test_folder_node_1 =
       AddFolder(mobile_node(), test_folder_title_1);
   const BookmarkNode* test_folder_node_2 =
@@ -152,11 +160,13 @@
   moved_nodes_.insert(test_folder_node_2);
   CreateSubDataSource();
 
-  auto visible_folder_nodes = [sub_data_source_ visibleFolderNodes];
-  ASSERT_EQ(2u, visible_folder_nodes.size());
-  EXPECT_NSEQ(base::SysUTF16ToNSString(visible_folder_nodes[0]->GetTitle()),
+  auto visible_folder_node_ids = [sub_data_source_ visibleFolderNodeIds];
+  ASSERT_EQ(2u, visible_folder_node_ids.size());
+  EXPECT_NSEQ(base::SysUTF16ToNSString(
+                  GetNodeByID(visible_folder_node_ids[0])->GetTitle()),
               @"Mobile bookmarks");
-  EXPECT_NSEQ(base::SysUTF16ToNSString(visible_folder_nodes[1]->GetTitle()),
+  EXPECT_NSEQ(base::SysUTF16ToNSString(
+                  GetNodeByID(visible_folder_node_ids[1])->GetTitle()),
               test_folder_title_1);
 }
 
@@ -170,11 +180,13 @@
   ChangeTitle(test_folder_node, test_folder_title_2);
 
   EXPECT_OCMOCK_VERIFY(mock_consumer_);
-  auto visible_folder_nodes = [sub_data_source_ visibleFolderNodes];
-  ASSERT_EQ(2u, visible_folder_nodes.size());
-  EXPECT_NSEQ(base::SysUTF16ToNSString(visible_folder_nodes[0]->GetTitle()),
+  auto visible_folder_node_ids = [sub_data_source_ visibleFolderNodeIds];
+  ASSERT_EQ(2u, visible_folder_node_ids.size());
+  EXPECT_NSEQ(base::SysUTF16ToNSString(
+                  GetNodeByID(visible_folder_node_ids[0])->GetTitle()),
               @"Mobile bookmarks");
-  EXPECT_NSEQ(base::SysUTF16ToNSString(visible_folder_nodes[1]->GetTitle()),
+  EXPECT_NSEQ(base::SysUTF16ToNSString(
+                  GetNodeByID(visible_folder_node_ids[1])->GetTitle()),
               test_folder_title_2);
 }
 
@@ -188,13 +200,16 @@
   AddFolder(test_folder_node_1, test_folder_title_2);
 
   EXPECT_OCMOCK_VERIFY(mock_consumer_);
-  auto visible_folder_nodes = [sub_data_source_ visibleFolderNodes];
-  ASSERT_EQ(3u, visible_folder_nodes.size());
-  EXPECT_NSEQ(base::SysUTF16ToNSString(visible_folder_nodes[0]->GetTitle()),
+  auto visible_folder_node_ids = [sub_data_source_ visibleFolderNodeIds];
+  ASSERT_EQ(3u, visible_folder_node_ids.size());
+  EXPECT_NSEQ(base::SysUTF16ToNSString(
+                  GetNodeByID(visible_folder_node_ids[0])->GetTitle()),
               @"Mobile bookmarks");
-  EXPECT_NSEQ(base::SysUTF16ToNSString(visible_folder_nodes[1]->GetTitle()),
+  EXPECT_NSEQ(base::SysUTF16ToNSString(
+                  GetNodeByID(visible_folder_node_ids[1])->GetTitle()),
               test_folder_title_1);
-  EXPECT_NSEQ(base::SysUTF16ToNSString(visible_folder_nodes[2]->GetTitle()),
+  EXPECT_NSEQ(base::SysUTF16ToNSString(
+                  GetNodeByID(visible_folder_node_ids[2])->GetTitle()),
               test_folder_title_2);
 }
 
@@ -216,11 +231,13 @@
   EXPECT_OCMOCK_VERIFY(mock_consumer_);
   ASSERT_EQ(test_folder_node_2,
             fake_parent_data_source_.bookmarkNodeDeletedArg);
-  auto visible_folder_nodes = [sub_data_source_ visibleFolderNodes];
-  ASSERT_EQ(2u, visible_folder_nodes.size());
-  EXPECT_NSEQ(base::SysUTF16ToNSString(visible_folder_nodes[0]->GetTitle()),
+  auto visible_folder_node_ids = [sub_data_source_ visibleFolderNodeIds];
+  ASSERT_EQ(2u, visible_folder_node_ids.size());
+  EXPECT_NSEQ(base::SysUTF16ToNSString(
+                  GetNodeByID(visible_folder_node_ids[0])->GetTitle()),
               @"Mobile bookmarks");
-  EXPECT_NSEQ(base::SysUTF16ToNSString(visible_folder_nodes[1]->GetTitle()),
+  EXPECT_NSEQ(base::SysUTF16ToNSString(
+                  GetNodeByID(visible_folder_node_ids[1])->GetTitle()),
               test_folder_title_1);
 }
 
@@ -236,10 +253,11 @@
   RemoveAllNodes();
 
   EXPECT_OCMOCK_VERIFY(mock_consumer_);
-  auto visible_folder_nodes = [sub_data_source_ visibleFolderNodes];
-  ASSERT_EQ(1u, visible_folder_nodes.size());
+  auto visible_folder_node_ids = [sub_data_source_ visibleFolderNodeIds];
+  ASSERT_EQ(1u, visible_folder_node_ids.size());
   // "Mobile Bookmarks" is a permanent node and thus always exists.
-  EXPECT_NSEQ(base::SysUTF16ToNSString(visible_folder_nodes[0]->GetTitle()),
+  EXPECT_NSEQ(base::SysUTF16ToNSString(
+                  GetNodeByID(visible_folder_node_ids[0])->GetTitle()),
               @"Mobile bookmarks");
 }
 
@@ -255,13 +273,16 @@
   MoveNode(test_folder_node_2, mobile_node());
 
   EXPECT_OCMOCK_VERIFY(mock_consumer_);
-  auto visible_folder_nodes = [sub_data_source_ visibleFolderNodes];
-  ASSERT_EQ(3u, visible_folder_nodes.size());
-  EXPECT_NSEQ(base::SysUTF16ToNSString(visible_folder_nodes[0]->GetTitle()),
+  auto visible_folder_node_ids = [sub_data_source_ visibleFolderNodeIds];
+  ASSERT_EQ(3u, visible_folder_node_ids.size());
+  EXPECT_NSEQ(base::SysUTF16ToNSString(
+                  GetNodeByID(visible_folder_node_ids[0])->GetTitle()),
               @"Mobile bookmarks");
-  EXPECT_NSEQ(base::SysUTF16ToNSString(visible_folder_nodes[1]->GetTitle()),
+  EXPECT_NSEQ(base::SysUTF16ToNSString(
+                  GetNodeByID(visible_folder_node_ids[1])->GetTitle()),
               test_folder_title_1);
-  EXPECT_NSEQ(base::SysUTF16ToNSString(visible_folder_nodes[2]->GetTitle()),
+  EXPECT_NSEQ(base::SysUTF16ToNSString(
+                  GetNodeByID(visible_folder_node_ids[2])->GetTitle()),
               test_folder_title_2);
 }
Loading diff…

Original Bug Report

reported by li...@chromium.org

Potential Use-After-Free in iOS BookmarksFolderChooserMediator during Sync Deletion

Flapjack, 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 Chrome Security team. Please see go/chrome-ai-generated-security-bugs-faq for more information.

Overview: A Use-After-Free (UAF) vulnerability exists in the iOS bookmarks folder chooser. If a parent folder is deleted remotely via Sync while a user is moving its child bookmark, the mediator retains a dangling raw pointer to the child because it only checks for the deletion of the root node. When the user completes the move, dereferencing the dangling pointer results in a UAF in the Browser Process.

Affected files:

  • ios/chrome/browser/bookmarks/folder_chooser/coordinator/bookmarks_folder_chooser_mediator.mm
  • ios/chrome/browser/bookmarks/ui_bundled/bookmark_utils_ios.mm

Estimated timestamp from git blame: 2026-01-09

Summary

A potential Use-After-Free (UAF) vulnerability exists in the iOS Bookmarks Folder Chooser. The vulnerability occurs when a user is in the process of moving a bookmark, and a remote sync event deletes an ancestor folder of that bookmark. The UI mediator fails to evict the descendant bookmarks from its selected items set, resulting in dangling raw pointers that are later dereferenced when the move operation is confirmed.

Technical Details

In iOS, when a user initiates a bookmark move, BookmarksFolderChooserMediator stores the selected nodes in a standard C++ set using raw pointers:

// ios/chrome/browser/bookmarks/folder_chooser/coordinator/bookmarks_folder_chooser_mediator.mm
@implementation BookmarksFolderChooserMediator {
  // ...
  std::set<const BookmarkNode*> _editedNodes;
}

When a remote device deletes a folder via Bookmark Sync, the iOS device processes the update on the UI thread. To prevent UI flicker, BookmarkDataTypeProcessor wraps the sync updates in ScopedRemoteUpdateBookmarks, which calls BookmarkModel::BeginExtensiveChanges(). This triggers BookmarkUndoService::ExtensiveBookmarkChangesBeginning(), which explicitly suspends undo tracking (undo_manager()->SuspendUndoTracking()).

The sync processor then calls BookmarkModel::RemoveChildAt to delete the folder. The BookmarkModel design dictates that observers are only notified about the root node being removed, not its descendants:

// components/bookmarks/browser/bookmark_model.cc
for (BookmarkModelObserver& observer : observers_) {
  observer.BookmarkNodeRemoved(parent, index, node, removed_urls, location);
}

The mediator receives this notification via its bridge:

- (void)bookmarkNodeDeleted:(const BookmarkNode*)bookmarkNode {
  if (_editedNodes.contains(bookmarkNode)) {
    _editedNodes.erase(bookmarkNode);
    // ... exit if empty ...
    return;
  }
  // ...
}

If the user is moving “Bookmark X” and the sync event deletes its parent “Folder A”, bookmarkNodeDeleted: is called with “Folder A”. Because _editedNodes contains “Bookmark X” (not “Folder A”), the check fails, and the pointer to “Bookmark X” remains in _editedNodes. The mediator lacks an ancestor check (e.g., deletedNode->HasAncestor(editedNode)).

Because undo tracking is currently suspended by the extensive sync changes, the unique_ptr holding the deleted folder is immediately discarded by UndoManager::AddUndoOperation. This instantly destroys the folder and its children, leaving the raw pointer in _editedNodes dangling. MiraclePtr (base::raw_ptr) does not protect raw pointers stored inside standard containers like std::set.

When the sync update finishes processing, the UI unblocks. If the user then selects a destination folder to complete the move, the dangling pointers are packaged into a std::vector and passed to bookmark_utils_ios::MoveBookmarksWithUndoSnackbar:

// ios/chrome/browser/bookmarks/ui_bundled/bookmark_utils_ios.mm
bool contains_a_folder =
    std::find_if(bookmarks_to_move.begin(), bookmarks_to_move.end(),
                 [](const BookmarkNode* node) {
                   return node->is_folder(); // UAF
                 }) != bookmarks_to_move.end();

Dereferencing the dangling pointer here, and subsequently in MoveBookmarks (node->parent()), triggers a Browser Process Use-After-Free.

Potential Reproduction Steps

Note: These are suggested steps based on static analysis.

  1. Sign in to Chrome on an iOS device and a Desktop device with the same account; enable Bookmark Sync.
  2. On both devices, ensure a folder named “Folder A” exists containing a bookmark named “Bookmark X”.
  3. On the iOS device, navigate to Bookmarks, tap Edit, select “Bookmark X”, and tap “Move”. The folder chooser UI appears.
  4. On the Desktop device, delete “Folder A”.
  5. Wait a moment for the deletion to sync to the iOS device. (The UI on iOS will remain open).
  6. On the iOS device, tap any destination folder (e.g., “Mobile Bookmarks”) to complete the move.
  7. The browser will crash due to a Use-After-Free in the browser process.

Suggested Fix

Update bookmarkNodeDeleted: in BookmarksFolderChooserMediator to properly handle the deletion of ancestor folders:

- (void)bookmarkNodeDeleted:(const BookmarkNode*)bookmarkNode {
  auto it = _editedNodes.begin();
  while (it != _editedNodes.end()) {
    if (*it == bookmarkNode || bookmarkNode->HasAncestor(*it)) {
      it = _editedNodes.erase(it);
    } else {
      ++it;
    }
  }
  if (_editedNodes.empty()) {
    [_delegate bookmarksFolderChooserMediatorWantsDismissal:self];
  }
}

Alternatively, consider using node IDs instead of raw pointers to track selected nodes across asynchronous operations.

Evaluated with Chrome root at commit: 4a3e9db74111a3c6c4b3acfd70050a05077cf27a

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.

View on issue tracker
On This Page