From f7faaf0f60da100d09c0556ff173159c3dce5faa Mon Sep 17 00:00:00 2001
From: Jan Keitel <jkeitel@google.com>
Date: Mon, 15 Jun 2026 08:01:58 -0700
Subject: [PATCH] Autofill: Fix UAF in PopupViewViews on Windows

Accessibility notifications can trigger a nested message loop that
destroys the view, but the calling functions continue to access the
freed `this` pointer.

This CL fixes the issue by:
1. Modifying `MaybeA11yFocusInformationalSuggestion()` to return a bool
   indicating whether the view survived the accessibility calls.
2. Updating callers `Show()` and `OnSuggestionsChanged()` to check this
   return value and return early if the view was destroyed.
3. Marking `TrackAndRun` as `[[nodiscard]]` to prevent future ignored
   return values.
4. Adding a unit test to verify the fix.

Note that this fix is not terribly robust against future code changes.
The linked bug contains a reference to a more fundamental solution
to popup lifetime issues.

TAG=agy
CONV=b8251de8-7593-4cd8-9dfa-d130c55f5101
Fixed: 523740781

Change-Id: Id478d96267f47e4e2b25e39dce3990f80c4c77d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7941589
Reviewed-by: Christoph Schwering <schwering@google.com>
Commit-Queue: Jan Keitel <jkeitel@google.com>
Cr-Commit-Position: refs/heads/main@{#1646809}
---

diff --git a/chrome/browser/ui/views/autofill/popup/popup_view_utils.h b/chrome/browser/ui/views/autofill/popup/popup_view_utils.h
index 1b24b95..eaa89037 100644
--- a/chrome/browser/ui/views/autofill/popup/popup_view_utils.h
+++ b/chrome/browser/ui/views/autofill/popup/popup_view_utils.h
@@ -198,8 +198,27 @@
 // This is typically used to prevent Use-After-Free (UAF) vulnerabilities on
 // Windows when firing platform accessibility events (e.g., focus or selection
 // changes).
+// Note that when `view` is `this`, the return value must be handled and must be
+// propagated:
+//
+// [[nodiscard]] bool PopupBaseView::Foo() {
+//   if (!TrackAndRun(this, callable)) {
+//     return false;
+//   }
+//   DoSomethingElse();
+// }
+//
+// // You may only drop the return value if you can guarantee that nothing
+// // accesses `this` or members of `this` after calling `Bar();`.
+// [[nodiscard]] bool PopupBaseView::Bar() {
+//   if (!Foo()) { return false; }
+//   DoSomethingElse();
+//   return true;
+// }
+//
+// TODO(crbug.com/524084900): Migrate to a more robust pattern.
 template <typename... Callables>
-bool TrackAndRun(views::View* view, Callables&&... callbacks) {
+[[nodiscard]] bool TrackAndRun(views::View* view, Callables&&... callbacks) {
   CHECK(view);
   views::ViewTracker tracker(view);
   bool alive = true;
diff --git a/chrome/browser/ui/views/autofill/popup/popup_view_views.cc b/chrome/browser/ui/views/autofill/popup/popup_view_views.cc
index c4b19c7..7d12e225 100644
--- a/chrome/browser/ui/views/autofill/popup/popup_view_views.cc
+++ b/chrome/browser/ui/views/autofill/popup/popup_view_views.cc
@@ -323,7 +323,9 @@
   MaybeAnnounceCurrentTabAndFootnote();
   MaybeAnnouncePasswordRecoveryPopup();
   MaybeAnnounceLoadingState();
-  MaybeA11yFocusInformationalSuggestion();
+  if (!MaybeA11yFocusInformationalSuggestion()) {
+    return false;
+  }
 
   return !CanActivate() || (GetWidget() && GetWidget()->IsActive());
 }
@@ -811,7 +813,9 @@
   MaybeAutoSelectSuggestion();
   MaybeAnnouncePasswordRecoveryPopup();
   MaybeAnnounceLoadingState();
-  MaybeA11yFocusInformationalSuggestion();
+  if (!MaybeA11yFocusInformationalSuggestion()) {
+    return;
+  }
   ShowIPHFeaturePromos();
 }
 
@@ -1690,21 +1694,23 @@
   return true;
 }
 
-void PopupViewViews::MaybeA11yFocusInformationalSuggestion() {
+bool PopupViewViews::MaybeA11yFocusInformationalSuggestion() {
   if (rows_.size() != 1) {
-    return;
+    return true;
   }
 
   if (auto* warning_view = std::get_if<PopupWarningView*>(&rows_[0]);
       warning_view && *warning_view) {
     PopupWarningView* view_ptr = *warning_view;
-    TrackAndRun(
+    return TrackAndRun(
         view_ptr, [this, view_ptr]() { NotifyAXSelection(*view_ptr); },
         [view_ptr]() {
           view_ptr->NotifyAccessibilityEventDeprecated(ax::mojom::Event::kFocus,
                                                        true);
         });
   }
+
+  return true;
 }
 
 base::WeakPtr<AutofillPopupView> PopupViewViews::GetWeakPtr() {
diff --git a/chrome/browser/ui/views/autofill/popup/popup_view_views.h b/chrome/browser/ui/views/autofill/popup/popup_view_views.h
index f984cc3a..0eb5a3b 100644
--- a/chrome/browser/ui/views/autofill/popup/popup_view_views.h
+++ b/chrome/browser/ui/views/autofill/popup/popup_view_views.h
@@ -329,7 +329,9 @@
   // the suggestion's message is being announced to the user by focusing the row
   // view (which must be selectable). Currently, only `PopupWarningView` is
   // supported.
-  void MaybeA11yFocusInformationalSuggestion();
+  // Returns true if the popup survived the accessibility event dispatch, false
+  // if it was destroyed. DO NOT access the popup if it has been destroyed.
+  [[nodiscard]] bool MaybeA11yFocusInformationalSuggestion();
 
   // Controller for this view.
   base::WeakPtr<AutofillPopupController> controller_ = nullptr;
diff --git a/chrome/browser/ui/views/autofill/popup/popup_view_views_unittest.cc b/chrome/browser/ui/views/autofill/popup/popup_view_views_unittest.cc
index 5f07448..c0ae055 100644
--- a/chrome/browser/ui/views/autofill/popup/popup_view_views_unittest.cc
+++ b/chrome/browser/ui/views/autofill/popup/popup_view_views_unittest.cc
@@ -154,6 +154,16 @@
         std::move(callback);
   }
 
+  void NotifyAXSelection(views::View& view) override {
+    if (destroy_on_notify_ax_selection_) {
+      GetWidget()->CloseNow();
+    } else {
+      PopupViewViews::NotifyAXSelection(view);
+    }
+  }
+
+  void DestroyOnNotifyAxSelection() { destroy_on_notify_ax_selection_ = true; }
+
  protected:
   gfx::Rect GetOptimalPositionAndPlaceArrowOnPopup(
       const gfx::Rect& element_bounds,
@@ -173,6 +183,7 @@
  private:
   GetOptimalPositionAndPlaceArrowOnPopupOverride
       get_optimal_position_and_place_arrow_on_popup_override_;
+  bool destroy_on_notify_ax_selection_ = false;
 };
 
 class PopupViewViewsTest : public ChromeViewsTestBase {
@@ -2750,6 +2761,15 @@
   EXPECT_EQ(1, counter.GetCount(ax::mojom::Event::kFocus, *row_view));
 }
 
+TEST_F(PopupViewViewsTest, WarningOnShow_DestroyOnA11yFocus) {
+  CreateView();
+  controller().set_suggestions({SuggestionType::kMixedFormMessage});
+  view().DestroyOnNotifyAxSelection();
+
+  // This should not crash!
+  ShowView(&view(), widget());
+}
+
 TEST_F(PopupViewViewsTest, Show_A11yAnnouncesPasswordRecovery) {
   const std::vector<Suggestion> suggestions = {
       Suggestion(SuggestionType::kBackupPasswordEntry),

diff --git a/chrome/browser/ui/views/autofill/popup/popup_view_views_unittest.cc b/chrome/browser/ui/views/autofill/popup/popup_view_views_unittest.cc
index 5f07448..c0ae055 100644
--- a/chrome/browser/ui/views/autofill/popup/popup_view_views_unittest.cc
+++ b/chrome/browser/ui/views/autofill/popup/popup_view_views_unittest.cc
@@ -154,6 +154,16 @@
         std::move(callback);
   }
 
+  void NotifyAXSelection(views::View& view) override {
+    if (destroy_on_notify_ax_selection_) {
+      GetWidget()->CloseNow();
+    } else {
+      PopupViewViews::NotifyAXSelection(view);
+    }
+  }
+
+  void DestroyOnNotifyAxSelection() { destroy_on_notify_ax_selection_ = true; }
+
  protected:
   gfx::Rect GetOptimalPositionAndPlaceArrowOnPopup(
       const gfx::Rect& element_bounds,
@@ -173,6 +183,7 @@
  private:
   GetOptimalPositionAndPlaceArrowOnPopupOverride
       get_optimal_position_and_place_arrow_on_popup_override_;
+  bool destroy_on_notify_ax_selection_ = false;
 };
 
 class PopupViewViewsTest : public ChromeViewsTestBase {
@@ -2750,6 +2761,15 @@
   EXPECT_EQ(1, counter.GetCount(ax::mojom::Event::kFocus, *row_view));
 }
 
+TEST_F(PopupViewViewsTest, WarningOnShow_DestroyOnA11yFocus) {
+  CreateView();
+  controller().set_suggestions({SuggestionType::kMixedFormMessage});
+  view().DestroyOnNotifyAxSelection();
+
+  // This should not crash!
+  ShowView(&view(), widget());
+}
+
 TEST_F(PopupViewViewsTest, Show_A11yAnnouncesPasswordRecovery) {
   const std::vector<Suggestion> suggestions = {
       Suggestion(SuggestionType::kBackupPasswordEntry),