From 9ba399777917241b7109962cf1eab7d70e64d3c7 Mon Sep 17 00:00:00 2001
From: Mohamed Amir Yosef <mamir@chromium.org>
Date: Wed, 10 Jun 2026 07:13:24 -0700
Subject: [PATCH] [TouchToFill] Fix UAF in TouchToFillController during destruction

During the destruction of `TouchToFillController` on Android, its member
variables are destroyed in the reverse order of their declaration:

1. `weak_ptr_factory_`

2. `visibility_controller_`

3. `ttf_delegate_`

When `ttf_delegate_` is destroyed, its destructor invokes
`authenticator_->Cancel()`. On Android, this synchronously runs a
failure callback that re-enters
`TouchToFillController::ActionCompleted`. Because `ActionCompleted` was
bound using `base::Unretained(this)`, it executes and accesses
`visibility_controller_` (which was already destroyed in step 2). This
triggers a Use-After-Destruction and a subsequent Use-After-Free
vulnerability.

This CL prevents the UAF by replacing `base::Unretained(this)` with
`weak_ptr_factory_.GetWeakPtr()` when binding the `ActionCompleted`
callback. Since `weak_ptr_factory_` is destroyed first, any re-entrant
callbacks fired during the destruction of `ttf_delegate_` or other
members are safely invalidated and ignored. Explanatory comments were
also added to prevent regressions.

Fixed: 521950423

Change-Id: I5a03851dc47d9cbcd38bc5f2d4f6b411c101b88d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7914725
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Ioana Treib <ioanap@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1644623}
---

diff --git a/chrome/browser/touch_to_fill/password_manager/touch_to_fill_controller.cc b/chrome/browser/touch_to_fill/password_manager/touch_to_fill_controller.cc
index af3aa93..183b8ca 100644
--- a/chrome/browser/touch_to_fill/password_manager/touch_to_fill_controller.cc
+++ b/chrome/browser/touch_to_fill/password_manager/touch_to_fill_controller.cc
@@ -206,10 +206,11 @@
     password_manager::metrics_util::LogFillSuggestionGroupedMatchAccepted(
         /*grouped_match_accepted=*/false);
   }
-  // Unretained is safe here because TouchToFillController owns the delegate.
+  // A WeakPtr is necessary because the delegate may trigger this callback
+  // during or after the destruction of this controller.
   ttf_delegate_->OnCredentialSelected(
       credential, base::BindOnce(&TouchToFillController::ActionCompleted,
-                                 base::Unretained(this)));
+                                 weak_ptr_factory_.GetWeakPtr()));
 }
 
 void TouchToFillController::OnAcknowledgementBeforeFillingReceived(
@@ -222,8 +223,8 @@
 
   switch (dismiss_reason) {
     case AcknowledgeGroupedCredentialSheetBridge::DismissReason::kAccept:
-      // Unretained is safe here because TouchToFillController owns the
-      // delegate.
+      // A WeakPtr is necessary because the delegate may trigger this callback
+      // during or after the destruction of this controller.
       ttf_delegate_->OnCredentialSelected(
           credential, base::BindOnce(&TouchToFillController::ActionCompleted,
                                      weak_ptr_factory_.GetWeakPtr()));
@@ -241,24 +242,28 @@
 void TouchToFillController::OnPasskeyCredentialSelected(
     const PasskeyCredential& credential) {
   view_.reset();
-  // Unretained is safe here because TouchToFillController owns the delegate.
+  // A WeakPtr is necessary because the delegate may trigger this callback
+  // during or after the destruction of this controller.
   ttf_delegate_->OnPasskeyCredentialSelected(
       credential, base::BindOnce(&TouchToFillController::ActionCompleted,
-                                 base::Unretained(this)));
+                                 weak_ptr_factory_.GetWeakPtr()));
 }
 
 void TouchToFillController::OnManagePasswordsSelected(bool passkeys_shown) {
   view_.reset();
-  // Unretained is safe here because TouchToFillController owns the delegate.
+  // A WeakPtr is necessary because the delegate may trigger this callback
+  // during or after the destruction of this controller.
   ttf_delegate_->OnManagePasswordsSelected(
       passkeys_shown, base::BindOnce(&TouchToFillController::ActionCompleted,
-                                     base::Unretained(this)));
+                                     weak_ptr_factory_.GetWeakPtr()));
 }
 
 void TouchToFillController::OnHybridSignInSelected() {
   view_.reset();
+  // A WeakPtr is necessary because the delegate may trigger this callback
+  // during or after the destruction of this controller.
   ttf_delegate_->OnHybridSignInSelected(base::BindOnce(
-      &TouchToFillController::ActionCompleted, base::Unretained(this)));
+      &TouchToFillController::ActionCompleted, weak_ptr_factory_.GetWeakPtr()));
 }
 
 void TouchToFillController::OnShowCredManSelected() {
@@ -271,9 +276,10 @@
   if (!ttf_delegate_) {
     return;
   }
-  // Unretained is safe here because TouchToFillController owns the delegate.
+  // A WeakPtr is necessary because the delegate may trigger this callback
+  // during or after the destruction of this controller.
   ttf_delegate_->OnCredManDismissed(base::BindOnce(
-      &TouchToFillController::ActionCompleted, base::Unretained(this)));
+      &TouchToFillController::ActionCompleted, weak_ptr_factory_.GetWeakPtr()));
 }
 
 void TouchToFillController::OnDismiss() {
@@ -284,9 +290,10 @@
     // PasswordSuggestionBottomSheetV2 is launched
     return;
   }
-  // Unretained is safe here because TouchToFillController owns the delegate.
+  // A WeakPtr is necessary because the delegate may trigger this callback
+  // during or after the destruction of this controller.
   ttf_delegate_->OnDismiss(base::BindOnce(
-      &TouchToFillController::ActionCompleted, base::Unretained(this)));
+      &TouchToFillController::ActionCompleted, weak_ptr_factory_.GetWeakPtr()));
 }
 
 Profile* TouchToFillController::GetProfile() {