From c17110d74d39df22037424c68fb620884d68f7e2 Mon Sep 17 00:00:00 2001
From: Mitsuru Oshima <[email protected]>
Date: Mon, 27 Jul 2026 12:11:59 -0700
Subject: [PATCH] Fix Use-After-Free in TransientWindowManager hide cascade

TransientWindowManager could be freed while updating child visibility
during parent hide cascade. This happens if the child has an observer
that synchronously deletes the parent. This adds weak ptr check inside
the iteration to prevent use after free.

TAG=agy
CONV=6a632318-b810-4c6e-a0e2-1135bf67758d

Fixed: 537838324
Change-Id: Ic5f4b3c32a03f75e95ce7c26dd2bc7f41b9ae097
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8143759
Commit-Queue: Mitsuru Oshima <[email protected]>
Reviewed-by: Achuith Bhandarkar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1668869}
---

diff --git a/ui/wm/core/transient_window_manager.cc b/ui/wm/core/transient_window_manager.cc
index ef261c6..a4b6b26 100644
--- a/ui/wm/core/transient_window_manager.cc
+++ b/ui/wm/core/transient_window_manager.cc
@@ -238,6 +238,9 @@
   if (window_ != window)
     return;
 
+  // Hiding transient children may delete parent as well.
+  base::WeakPtr<TransientWindowManager> weak_this = weak_factory_.GetWeakPtr();
+
   // If the window has transient children, updates the transient children's
   // visiblity as well.
   // WindowTracker is used because child window
@@ -246,6 +249,10 @@
   while (!tracker.windows().empty())
     GetOrCreate(tracker.Pop())->UpdateTransientChildVisibility(visible);
 
+  if (!weak_this) {
+    return;
+  }
+
   // Remember the show request in |show_on_parent_visible_| and hide it again
   // if the following conditions are met
   // - |parent_controls_visibility| is set to true.
diff --git a/ui/wm/core/transient_window_manager_unittest.cc b/ui/wm/core/transient_window_manager_unittest.cc
index 28c7de54..9e901c0b 100644
--- a/ui/wm/core/transient_window_manager_unittest.cc
+++ b/ui/wm/core/transient_window_manager_unittest.cc
@@ -728,4 +728,60 @@
   child->Show();
 }
 
+namespace {
+
+class DeleteParentOnHideObserver : public aura::WindowObserver {
+ public:
+  DeleteParentOnHideObserver(aura::Window* child,
+                             std::unique_ptr<aura::Window> parent)
+      : child_(child), parent_(std::move(parent)) {
+    child_->AddObserver(this);
+  }
+  ~DeleteParentOnHideObserver() override {
+    if (child_) {
+      child_->RemoveObserver(this);
+    }
+  }
+  // WindowObserver:
+  void OnWindowVisibilityChanged(aura::Window* window,
+                                 bool visible) override {
+    if (window == child_ && !visible) {
+      parent_.reset();
+    }
+  }
+  void OnWindowDestroyed(aura::Window* window) override {
+    if (window == child_) {
+      child_ = nullptr;
+    }
+  }
+
+ private:
+  raw_ptr<aura::Window> child_;
+  std::unique_ptr<aura::Window> parent_;
+};
+
+}  // namespace
+
+// Tests that there is no UAF if a window is destroyed while its transient
+// children are being hidden in a cascade update.
+TEST_F(TransientWindowManagerTest,
+       ParentDestroyedDuringTransientChildHideCascade) {
+  std::unique_ptr<aura::Window> parent = CreateTestWindow(
+      {.parent = root_window(), .bounds = {100, 100}, .window_id = 0});
+  aura::Window* parent_ptr = parent.get();
+
+  std::unique_ptr<aura::Window> child_ptr = CreateTestWindow(
+      {.parent = root_window(), .bounds = {100, 100}, .window_id = 1});
+  aura::Window* child = child_ptr.release();
+
+  TransientWindowManager::GetOrCreate(child)->set_parent_controls_visibility(
+      true);
+  AddTransientChild(parent_ptr, child);
+
+  auto observer =
+      std::make_unique<DeleteParentOnHideObserver>(child, std::move(parent));
+
+  parent_ptr->Hide();
+}
+
 }  // namespace wm

diff --git a/ui/wm/core/transient_window_manager_unittest.cc b/ui/wm/core/transient_window_manager_unittest.cc
index 28c7de54..9e901c0b 100644
--- a/ui/wm/core/transient_window_manager_unittest.cc
+++ b/ui/wm/core/transient_window_manager_unittest.cc
@@ -728,4 +728,60 @@
   child->Show();
 }
 
+namespace {
+
+class DeleteParentOnHideObserver : public aura::WindowObserver {
+ public:
+  DeleteParentOnHideObserver(aura::Window* child,
+                             std::unique_ptr<aura::Window> parent)
+      : child_(child), parent_(std::move(parent)) {
+    child_->AddObserver(this);
+  }
+  ~DeleteParentOnHideObserver() override {
+    if (child_) {
+      child_->RemoveObserver(this);
+    }
+  }
+  // WindowObserver:
+  void OnWindowVisibilityChanged(aura::Window* window,
+                                 bool visible) override {
+    if (window == child_ && !visible) {
+      parent_.reset();
+    }
+  }
+  void OnWindowDestroyed(aura::Window* window) override {
+    if (window == child_) {
+      child_ = nullptr;
+    }
+  }
+
+ private:
+  raw_ptr<aura::Window> child_;
+  std::unique_ptr<aura::Window> parent_;
+};
+
+}  // namespace
+
+// Tests that there is no UAF if a window is destroyed while its transient
+// children are being hidden in a cascade update.
+TEST_F(TransientWindowManagerTest,
+       ParentDestroyedDuringTransientChildHideCascade) {
+  std::unique_ptr<aura::Window> parent = CreateTestWindow(
+      {.parent = root_window(), .bounds = {100, 100}, .window_id = 0});
+  aura::Window* parent_ptr = parent.get();
+
+  std::unique_ptr<aura::Window> child_ptr = CreateTestWindow(
+      {.parent = root_window(), .bounds = {100, 100}, .window_id = 1});
+  aura::Window* child = child_ptr.release();
+
+  TransientWindowManager::GetOrCreate(child)->set_parent_controls_visibility(
+      true);
+  AddTransientChild(parent_ptr, child);
+
+  auto observer =
+      std::make_unique<DeleteParentOnHideObserver>(child, std::move(parent));
+
+  parent_ptr->Hide();
+}
+
 }  // namespace wm