From cd32c341b8b3cb3d262e105d394aab673601e6a4 Mon Sep 17 00:00:00 2001
From: Huiting Yu <[email protected]>
Date: Tue, 11 Aug 2026 08:15:18 -0700
Subject: [PATCH] [iOS][Bookmarks] Snapshot tab URL when bookmarks UI is presented

The bookmarklet origin check captured the active tab's URL when the user
tapped a bookmark row, then compared it to the URL after the dismissal
animation. However, the underlying tab can navigate while the bookmark
browser form sheet is covering it, before any row is tapped, in which
case the snapshot already reflects the new URL and the check trivially
passes.

Capture the URL in -presentBookmarksAtDisplayedFolderNode: instead, so
any navigation that occurs while the bookmark browser is displayed is
detected and blocked from executing bookmarklets.

Added an EarlGrey regression test that navigates the underlying tab while
the bookmark browser is open and verifies the bookmarklet does not execute
on the new page.

Fixed: 517581661
Change-Id: I0f404c04f39ca3622b9dd840476689aa470b571d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8230501
Auto-Submit: Huiting Yu <[email protected]>
Commit-Queue: Huiting Yu <[email protected]>
Reviewed-by: Arthur Milchior <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1677220}
---

diff --git a/ios/chrome/browser/bookmarks/test/bookmarks_security_egtest.mm b/ios/chrome/browser/bookmarks/test/bookmarks_security_egtest.mm
index 7c96ac5..f036d56bd 100644
--- a/ios/chrome/browser/bookmarks/test/bookmarks_security_egtest.mm
+++ b/ios/chrome/browser/bookmarks/test/bookmarks_security_egtest.mm
@@ -67,12 +67,53 @@
   // Trigger background navigation programmatically immediately after tapping
   // the bookmarklet.
   GURL sensitiveURL = self.testServer->GetURL("/toctou_sensitive.html");
-  [ChromeEarlGrey loadURL:sensitiveURL];
+  [ChromeEarlGrey loadURL:sensitiveURL waitForCompletion:NO];
+  [ChromeEarlGrey waitForPageToFinishLoading];
 
   // Tapping the bookmarklet should close the bookmarks UI.
-  [[EarlGrey selectElementWithMatcher:grey_accessibilityID(
-                                          kBookmarksHomeTableViewIdentifier)]
-      assertWithMatcher:grey_nil()];
+  [ChromeEarlGrey waitForUIElementToDisappearWithMatcher:
+                      grey_accessibilityID(kBookmarksHomeTableViewIdentifier)];
+
+  // Verify that the bookmarklet was NOT executed on the sensitive page.
+  [ChromeEarlGrey waitForWebStateContainingText:"Not executed"];
+}
+
+// Tests that a bookmarklet is blocked if the active tab navigated to a
+// different origin in the background while the Bookmarks UI was open, before
+// the bookmarklet was tapped.
+- (void)testBookmarkletTOCTOUMitigationWhileBookmarksOpen {
+  // Add the bookmarklet programmatically.
+  NSString* bookmarkletURL =
+      @"javascript:document.getElementById('result').innerText='EXECUTED';";
+  [BookmarkEarlGrey addBookmarkWithTitle:@"TOCTOU_Bookmarklet"
+                                     URL:bookmarkletURL
+                               inStorage:BookmarkStorageType::kLocalOrSyncable];
+
+  // Start the test server.
+  GREYAssertTrue(self.testServer->Start(), @"Test server failed to start.");
+
+  // Load the attacker page.
+  GURL attackerURL = self.testServer->GetURL("/toctou_attacker.html");
+  [ChromeEarlGrey loadURL:attackerURL];
+
+  // Open Bookmarks UI using the real UI helper.
+  [BookmarkEarlGreyUI openBookmarks];
+  [BookmarkEarlGreyUI openMobileBookmarks];
+
+  // Trigger background navigation while the Bookmarks UI is open, BEFORE
+  // tapping the bookmarklet.
+  GURL sensitiveURL = self.testServer->GetURL("/toctou_sensitive.html");
+  [ChromeEarlGrey loadURL:sensitiveURL waitForCompletion:NO];
+  [ChromeEarlGrey waitForPageToFinishLoading];
+
+  // Tap the bookmarklet in the UI.
+  [[EarlGrey selectElementWithMatcher:TappableBookmarkNodeWithLabel(
+                                          @"TOCTOU_Bookmarklet")]
+      performAction:grey_tap()];
+
+  // Tapping the bookmarklet should close the bookmarks UI.
+  [ChromeEarlGrey waitForUIElementToDisappearWithMatcher:
+                      grey_accessibilityID(kBookmarksHomeTableViewIdentifier)];
 
   // Verify that the bookmarklet was NOT executed on the sensitive page.
   [ChromeEarlGrey waitForWebStateContainingText:"Not executed"];
@@ -105,9 +146,8 @@
       performAction:grey_tap()];
 
   // Tapping the bookmarklet should close the bookmarks UI.
-  [[EarlGrey selectElementWithMatcher:grey_accessibilityID(
-                                          kBookmarksHomeTableViewIdentifier)]
-      assertWithMatcher:grey_nil()];
+  [ChromeEarlGrey waitForUIElementToDisappearWithMatcher:
+                      grey_accessibilityID(kBookmarksHomeTableViewIdentifier)];
 
   // Verify that the bookmarklet WAS executed successfully on the page.
   [ChromeEarlGrey waitForWebStateContainingText:"EXECUTED"];
@@ -136,9 +176,8 @@
       performAction:grey_tap()];
 
   // Tapping the bookmarklet should close the bookmarks UI successfully.
-  [[EarlGrey selectElementWithMatcher:grey_accessibilityID(
-                                          kBookmarksHomeTableViewIdentifier)]
-      assertWithMatcher:grey_nil()];
+  [ChromeEarlGrey waitForUIElementToDisappearWithMatcher:
+                      grey_accessibilityID(kBookmarksHomeTableViewIdentifier)];
 }
 
 // Tests that opening a bookmark when there are no active tabs (e.g. all tabs
@@ -165,9 +204,8 @@
       performAction:grey_tap()];
 
   // Tapping the bookmark should close the bookmarks UI.
-  [[EarlGrey selectElementWithMatcher:grey_accessibilityID(
-                                          kBookmarksHomeTableViewIdentifier)]
-      assertWithMatcher:grey_nil()];
+  [ChromeEarlGrey waitForUIElementToDisappearWithMatcher:
+                      grey_accessibilityID(kBookmarksHomeTableViewIdentifier)];
 
   // Verify that a tab was opened with the bookmark's URL.
   [ChromeEarlGrey waitForMainTabCount:1];
diff --git a/ios/chrome/browser/bookmarks/ui_bundled/home/bookmarks_coordinator.mm b/ios/chrome/browser/bookmarks/ui_bundled/home/bookmarks_coordinator.mm
index 572c708..049ec95 100644
--- a/ios/chrome/browser/bookmarks/ui_bundled/home/bookmarks_coordinator.mm
+++ b/ios/chrome/browser/bookmarks/ui_bundled/home/bookmarks_coordinator.mm
@@ -132,6 +132,11 @@
   // Coordinator to display the "Set a reminder" UI for the user's selected
   // bookmark.
   ReminderNotificationsCoordinator* _reminderNotificationsCoordinator;
+
+  // The last committed URL of the active `WebState` when the bookmarks UI
+  // was presented. Used to prevent Universal Cross-Site Scripting (UXSS)
+  // if the underlying tab navigates while bookmarks UI is open.
+  GURL _lastCommittedURLBeforePresentation;
 }
 
 @synthesize sceneHandler = _sceneHandler;
@@ -185,6 +190,7 @@
   _currentProfile = nullptr;
   _bookmarkModel = nullptr;
   _mediator = nil;
+  _lastCommittedURLBeforePresentation = GURL();
   CHECK_EQ(PresentedState::NONE, self.currentPresentedState,
            base::NotFatalUntil::M152);
   CHECK(!self.bookmarkEditorCoordinator, base::NotFatalUntil::M152)
@@ -331,14 +337,7 @@
             _currentProfile.get()));
   }
 
-  GURL urlBeforeDismissal;
-  if (self.browser && self.browser->GetWebStateList()) {
-    web::WebState* activeWebState =
-        self.browser->GetWebStateList()->GetActiveWebState();
-    if (activeWebState) {
-      urlBeforeDismissal = activeWebState->GetLastCommittedURL();
-    }
-  }
+  GURL urlBeforePresentation = _lastCommittedURLBeforePresentation;
 
   // First the bookmark view should be dismissed to have the animation, and
   // the URLs should be opened.
@@ -346,13 +345,13 @@
   // bookmark view without animation.
   ProceduralBlock dismissCompletion = base::CallbackToBlock(base::BindOnce(
       [](__weak __typeof(self) weakSelf, std::vector<GURL> urls_to_open,
-         BOOL in_incognito, BOOL new_tab, GURL url_before_dismissal) {
+         BOOL in_incognito, BOOL new_tab, GURL url_before_presentation) {
         [weakSelf openUrls:urls_to_open
-                   inIncognito:in_incognito
-                        newTab:new_tab
-            urlBeforeDismissal:url_before_dismissal];
+                      inIncognito:in_incognito
+                           newTab:new_tab
+            urlBeforePresentation:url_before_presentation];
       },
-      self, urlsToOpen, inIncognito, newTab, urlBeforeDismissal));
+      self, urlsToOpen, inIncognito, newTab, urlBeforePresentation));
 
   if (self.baseViewController.presentedViewController) {
     [self.baseViewController dismissViewControllerAnimated:animated
@@ -388,6 +387,7 @@
   self.bookmarkNavigationController.presentationController.delegate = nil;
   self.bookmarkNavigationController.delegate = nil;
   self.bookmarkNavigationController = nil;
+  _lastCommittedURLBeforePresentation = GURL();
   self.currentPresentedState = PresentedState::NONE;
 }
 
@@ -495,13 +495,12 @@
 //   foreground, others are opened in background tabs.
 // `inIncognito`: Whether the URLs should be opened in an incognito tab.
 // `newTab`: Whether the URLs should be forced to open in a new tab.
-// `urlBeforeDismissal`: The GURL of the active web state before the bookmarks
-//   UI dismissal animation started. Used to prevent Universal Cross-Site

diff --git a/ios/chrome/browser/bookmarks/test/bookmarks_security_egtest.mm b/ios/chrome/browser/bookmarks/test/bookmarks_security_egtest.mm
index 7c96ac5..f036d56bd 100644
--- a/ios/chrome/browser/bookmarks/test/bookmarks_security_egtest.mm
+++ b/ios/chrome/browser/bookmarks/test/bookmarks_security_egtest.mm
@@ -67,12 +67,53 @@
   // Trigger background navigation programmatically immediately after tapping
   // the bookmarklet.
   GURL sensitiveURL = self.testServer->GetURL("/toctou_sensitive.html");
-  [ChromeEarlGrey loadURL:sensitiveURL];
+  [ChromeEarlGrey loadURL:sensitiveURL waitForCompletion:NO];
+  [ChromeEarlGrey waitForPageToFinishLoading];
 
   // Tapping the bookmarklet should close the bookmarks UI.
-  [[EarlGrey selectElementWithMatcher:grey_accessibilityID(
-                                          kBookmarksHomeTableViewIdentifier)]
-      assertWithMatcher:grey_nil()];
+  [ChromeEarlGrey waitForUIElementToDisappearWithMatcher:
+                      grey_accessibilityID(kBookmarksHomeTableViewIdentifier)];
+
+  // Verify that the bookmarklet was NOT executed on the sensitive page.
+  [ChromeEarlGrey waitForWebStateContainingText:"Not executed"];
+}
+
+// Tests that a bookmarklet is blocked if the active tab navigated to a
+// different origin in the background while the Bookmarks UI was open, before
+// the bookmarklet was tapped.
+- (void)testBookmarkletTOCTOUMitigationWhileBookmarksOpen {
+  // Add the bookmarklet programmatically.
+  NSString* bookmarkletURL =
+      @"javascript:document.getElementById('result').innerText='EXECUTED';";
+  [BookmarkEarlGrey addBookmarkWithTitle:@"TOCTOU_Bookmarklet"
+                                     URL:bookmarkletURL
+                               inStorage:BookmarkStorageType::kLocalOrSyncable];
+
+  // Start the test server.
+  GREYAssertTrue(self.testServer->Start(), @"Test server failed to start.");
+
+  // Load the attacker page.
+  GURL attackerURL = self.testServer->GetURL("/toctou_attacker.html");
+  [ChromeEarlGrey loadURL:attackerURL];
+
+  // Open Bookmarks UI using the real UI helper.
+  [BookmarkEarlGreyUI openBookmarks];
+  [BookmarkEarlGreyUI openMobileBookmarks];
+
+  // Trigger background navigation while the Bookmarks UI is open, BEFORE
+  // tapping the bookmarklet.
+  GURL sensitiveURL = self.testServer->GetURL("/toctou_sensitive.html");
+  [ChromeEarlGrey loadURL:sensitiveURL waitForCompletion:NO];
+  [ChromeEarlGrey waitForPageToFinishLoading];
+
+  // Tap the bookmarklet in the UI.
+  [[EarlGrey selectElementWithMatcher:TappableBookmarkNodeWithLabel(
+                                          @"TOCTOU_Bookmarklet")]
+      performAction:grey_tap()];
+
+  // Tapping the bookmarklet should close the bookmarks UI.
+  [ChromeEarlGrey waitForUIElementToDisappearWithMatcher:
+                      grey_accessibilityID(kBookmarksHomeTableViewIdentifier)];
 
   // Verify that the bookmarklet was NOT executed on the sensitive page.
   [ChromeEarlGrey waitForWebStateContainingText:"Not executed"];
@@ -105,9 +146,8 @@
       performAction:grey_tap()];
 
   // Tapping the bookmarklet should close the bookmarks UI.
-  [[EarlGrey selectElementWithMatcher:grey_accessibilityID(
-                                          kBookmarksHomeTableViewIdentifier)]
-      assertWithMatcher:grey_nil()];
+  [ChromeEarlGrey waitForUIElementToDisappearWithMatcher:
+                      grey_accessibilityID(kBookmarksHomeTableViewIdentifier)];
 
   // Verify that the bookmarklet WAS executed successfully on the page.
   [ChromeEarlGrey waitForWebStateContainingText:"EXECUTED"];
@@ -136,9 +176,8 @@
       performAction:grey_tap()];
 
   // Tapping the bookmarklet should close the bookmarks UI successfully.
-  [[EarlGrey selectElementWithMatcher:grey_accessibilityID(
-                                          kBookmarksHomeTableViewIdentifier)]
-      assertWithMatcher:grey_nil()];
+  [ChromeEarlGrey waitForUIElementToDisappearWithMatcher:
+                      grey_accessibilityID(kBookmarksHomeTableViewIdentifier)];
 }
 
 // Tests that opening a bookmark when there are no active tabs (e.g. all tabs
@@ -165,9 +204,8 @@
       performAction:grey_tap()];
 
   // Tapping the bookmark should close the bookmarks UI.
-  [[EarlGrey selectElementWithMatcher:grey_accessibilityID(
-                                          kBookmarksHomeTableViewIdentifier)]
-      assertWithMatcher:grey_nil()];
+  [ChromeEarlGrey waitForUIElementToDisappearWithMatcher:
+                      grey_accessibilityID(kBookmarksHomeTableViewIdentifier)];
 
   // Verify that a tab was opened with the bookmark's URL.
   [ChromeEarlGrey waitForMainTabCount:1];