From b867f0ed3dc1854a05561e526ccb2442d4fd78e4 Mon Sep 17 00:00:00 2001
From: David Roger <[email protected]>
Date: Wed, 13 May 2026 06:12:54 -0700
Subject: [PATCH] [sync] Reset observation in OnSyncConfirmationUIClosed

This is a fix for a bug that may happen when triggering two sync dialogs
at the same time. Opening a second dialogs closes the first dialog, and,
if the widget closes synchronously, OnSyncConfirmationUIClosed() can be
re-entrant (it is called again from
`signin_view_controller()->CloseModalSignin()`) and ultimately this
results in a UaF.

`OnSyncConfirmationUIClosed()` is meant to be called only once, so this
CL unregisters the observer immediately after it is called. This should
fix the re-entrant calls, and as a consequence the UaF too.

There is no regression test, because in all my attempts the widget was
destroying asynchronously, and I think the bug can only trigger if the
widget is destroyed synchronously. It is possible that the widget
implementation changed since the bug was filed, and it no longer
happens.

Fixed: 361375787
Change-Id: I1c5605a157a3ceba058093595dc104c026afc3a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7840682
Reviewed-by: Mihai Sardarescu <[email protected]>
Commit-Queue: David Roger <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1629915}
---

diff --git a/chrome/browser/ui/webui/signin/turn_sync_on_helper_delegate_impl.cc b/chrome/browser/ui/webui/signin/turn_sync_on_helper_delegate_impl.cc
index ae76c04..330d07a 100644
--- a/chrome/browser/ui/webui/signin/turn_sync_on_helper_delegate_impl.cc
+++ b/chrome/browser/ui/webui/signin/turn_sync_on_helper_delegate_impl.cc
@@ -201,6 +201,7 @@
 
 void TurnSyncOnHelperDelegateImpl::OnSyncConfirmationUIClosed(
     LoginUIService::SyncConfirmationUIClosedResult result) {
+  scoped_login_ui_service_observation_.Reset();
   DCHECK(sync_confirmation_callback_);
   // Treat closing the ui as an implicit ABORT_SYNC action.
   if (result == LoginUIService::UI_CLOSED) {