From 799efabee5c4e6d4466adad750552059a807feab Mon Sep 17 00:00:00 2001
From: Guillaume Jenkins <[email protected]>
Date: Thu, 11 Jun 2026 13:13:19 -0700
Subject: [PATCH] [iOS][OverflowMenu] Better cleanup in Overflow Menu

This CL fixes a potential UAF vulnerability caused by a strong retention
cycle keeping OverflowMenuMediator alive after coordinator teardown,
leaving it with dangling pointers to C++ browser agents.

Fixes implemented:

1. Break the strong retention cycle in PopupMenuCoordinator (-stop and
   -dismissPopupMenuAnimated:) by clearing self.contentBlockerMediator.
2. Explicitly null out all assigned C++ properties in
   OverflowMenuMediator's -disconnect.
3. Set self.popupMenuCoordinator to nil in BrowserViewController's
   -shutdown.
4. Made BrowserContentMediator's consumer property weak.

Bug: 518243858
Change-Id: If2d2ecab1c09bb45f0214a42beaa46e70ecc4059
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7893606
Reviewed-by: Robbie Gibson <[email protected]>
Reviewed-by: Gauthier Ambard <[email protected]>
Commit-Queue: Guillaume Jenkins <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1645562}
---

diff --git a/ios/chrome/browser/browser_content/ui_bundled/browser_content_mediator.h b/ios/chrome/browser/browser_content/ui_bundled/browser_content_mediator.h
index e69af676..28c9a11c 100644
--- a/ios/chrome/browser/browser_content/ui_bundled/browser_content_mediator.h
+++ b/ios/chrome/browser/browser_content/ui_bundled/browser_content_mediator.h
@@ -24,7 +24,7 @@
 - (instancetype)init NS_UNAVAILABLE;
 
 // The consumer.  Setting to a new value configures the new consumer.
-@property(nonatomic, strong) id<BrowserContentConsumer> consumer;
+@property(nonatomic, weak) id<BrowserContentConsumer> consumer;
 
 @end
 
diff --git a/ios/chrome/browser/browser_view/ui_bundled/browser_view_controller.mm b/ios/chrome/browser/browser_view/ui_bundled/browser_view_controller.mm
index 76b3d804..be5ce98 100644
--- a/ios/chrome/browser/browser_view/ui_bundled/browser_view_controller.mm
+++ b/ios/chrome/browser/browser_view/ui_bundled/browser_view_controller.mm
@@ -878,6 +878,7 @@
   _layoutState = nil;
   [[NSNotificationCenter defaultCenter] removeObserver:self];
   _bookmarksCoordinator = nil;
+  self.popupMenuCoordinator = nil;
 
   // Clears the pointer to C++ objects.
   _urlLoadingBrowserAgent = nullptr;
diff --git a/ios/chrome/browser/popup_menu/coordinator/popup_menu_coordinator.mm b/ios/chrome/browser/popup_menu/coordinator/popup_menu_coordinator.mm
index 09d735aa..64678bb 100644
--- a/ios/chrome/browser/popup_menu/coordinator/popup_menu_coordinator.mm
+++ b/ios/chrome/browser/popup_menu/coordinator/popup_menu_coordinator.mm
@@ -172,6 +172,8 @@
   [self.browser->GetCommandDispatcher() stopDispatchingToTarget:self];
   [self.overflowMenuMediator disconnect];
   self.overflowMenuMediator = nil;
+  self.contentBlockerMediator.consumer = nil;
+  self.contentBlockerMediator = nil;
 }
 
 #pragma mark - Public
@@ -496,6 +498,8 @@
     _overflowMenuOrderer = nil;
     [self.overflowMenuMediator disconnect];
     self.overflowMenuMediator = nil;
+    self.contentBlockerMediator.consumer = nil;
+    self.contentBlockerMediator = nil;
   }
 }
 
diff --git a/ios/chrome/browser/popup_menu/overflow_menu/coordinator/overflow_menu_mediator.mm b/ios/chrome/browser/popup_menu/overflow_menu/coordinator/overflow_menu_mediator.mm
index 479a8c3..f4f4953 100644
--- a/ios/chrome/browser/popup_menu/overflow_menu/coordinator/overflow_menu_mediator.mm
+++ b/ios/chrome/browser/popup_menu/overflow_menu/coordinator/overflow_menu_mediator.mm
@@ -358,6 +358,13 @@
   _authServiceObserverBridge.reset();
   _identityManager = nullptr;
   _identityManagerObserverBridge.reset();
+
+  self.navigationAgent = nullptr;
+  self.browserPolicyConnector = nullptr;
+  self.promosManager = nullptr;
+  self.readingListBrowserAgent = nullptr;
+  self.tabBasedIPHBrowserAgent = nullptr;
+  self.templateURLService = nullptr;
 }
 
 #pragma mark - Property getters/setters
@@ -2502,15 +2509,25 @@
 - (void)reload {
   RecordAction(UserMetricsAction("MobileMenuReload"));
   self.tabBasedIPHBrowserAgent->NotifyMultiGestureRefreshEvent();
+  // Dismissing the menu disconnects the mediator, so save anything cleaned up
+  // there.
+  WebNavigationBrowserAgent* navigationAgent = self.navigationAgent;
   [self dismissMenu];
-  self.navigationAgent->Reload();
+  if (navigationAgent) {
+    navigationAgent->Reload();
+  }
 }
 
 // Dismisses the menu and stops the current page load.
 - (void)stopLoading {
   RecordAction(UserMetricsAction("MobileMenuStop"));
+  // Dismissing the menu disconnects the mediator, so save anything cleaned up
+  // there.
+  WebNavigationBrowserAgent* navigationAgent = self.navigationAgent;
   [self dismissMenu];
-  self.navigationAgent->StopLoading();
+  if (navigationAgent) {
+    navigationAgent->StopLoading();
+  }
 }
 
 // Dismisses the menu and opens a new tab.
@@ -2593,8 +2610,13 @@
 // Dismisses the menu and requests the desktop version of the current page
 - (void)requestDesktopSite {
   RecordAction(UserMetricsAction("MobileMenuRequestDesktopSite"));
+  // Dismissing the menu disconnects the mediator, so save anything cleaned up
+  // there.
+  WebNavigationBrowserAgent* navigationAgent = self.navigationAgent;
   [self dismissMenu];
-  self.navigationAgent->RequestDesktopSite();
+  if (navigationAgent) {
+    navigationAgent->RequestDesktopSite();
+  }
   [self.helpHandler
       presentInProductHelpWithType:InProductHelpType::kDefaultSiteView];
 }
@@ -2613,8 +2635,13 @@
 // Dismisses the menu and requests the mobile version of the current page
 - (void)requestMobileSite {
   RecordAction(UserMetricsAction("MobileMenuRequestMobileSite"));
+  // Dismissing the menu disconnects the mediator, so save anything cleaned up
+  // there.
+  WebNavigationBrowserAgent* navigationAgent = self.navigationAgent;
   [self dismissMenu];
-  self.navigationAgent->RequestMobileSite();
+  if (navigationAgent) {
+    navigationAgent->RequestMobileSite();
+  }
 }
 
 // Dismisses the menu and opens Find In Page