From 21eeb0ad8f75440b004552bf7e6f4abf34931999 Mon Sep 17 00:00:00 2001
From: fikus <[email protected]>
Date: Tue, 15 Sep 2026 09:05:51 -0700
Subject: [PATCH] [NativeWidgetAura] Prevent potential UAF in Close()

Guard against potential use-after-free in NativeWidgetAura::Close.

Calling Hide() may destroy the window, which can trigger a call to
`delete this` in the widget. Use a WeakPtr to check if `this` has been
freed.

Bug: 553134101
Fixed: 553134101
Change-Id: I6f6608e77c6a7181682d25efa391e4ad6a6a6964
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8407987
Reviewed-by: Dana Fried <[email protected]>
Reviewed-by: Allen Bauer <[email protected]>
Commit-Queue: Eric Fikus <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1698142}
---
diff --git a/ui/views/widget/native_widget_aura.cc b/ui/views/widget/native_widget_aura.cc
index e8aaee6..43c789c 100644
--- a/ui/views/widget/native_widget_aura.cc
+++ b/ui/views/widget/native_widget_aura.cc
@@ -743,14 +743,17 @@
   DCHECK(window_ ||
          ownership_ == Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET ||
          ownership_ == Widget::InitParams::CLIENT_OWNS_WIDGET);
+  auto weak_this = weak_factory.GetWeakPtr();
   if (window_) {
     Hide();
+    if (!weak_this || !window_) {
+      return;
+    }
     window_->SetProperty(aura::client::kModalKey, ui::mojom::ModalType::kNone);
   }
 
   base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
-      FROM_HERE,
-      base::BindOnce(&NativeWidgetAura::CloseNow, weak_factory.GetWeakPtr()));
+      FROM_HERE, base::BindOnce(&NativeWidgetAura::CloseNow, weak_this));
 }
 
 void NativeWidgetAura::CloseNow() {
diff --git a/ui/views/widget/native_widget_aura_unittest.cc b/ui/views/widget/native_widget_aura_unittest.cc
index 77c1de9..0ccbf8f 100644
--- a/ui/views/widget/native_widget_aura_unittest.cc
+++ b/ui/views/widget/native_widget_aura_unittest.cc
@@ -1223,4 +1223,42 @@
   native_widget_->UpdateVisualState();
 }
 
+namespace {
+
+// An aura::WindowObserver that deletes the window when it is hidden.
+class DeleteWindowOnHide : public aura::WindowObserver {
+ public:
+  explicit DeleteWindowOnHide(aura::Window* window) {
+    observation_.Observe(window);
+  }
+
+  // aura::WindowObserver:
+  void OnWindowVisibilityChanged(aura::Window* window, bool visible) override {
+    if (!visible && observation_.IsObservingSource(window)) {
+      observation_.Reset();
+      delete window;
+    }
+  }
+
+ private:
+  base::ScopedObservation<aura::Window, aura::WindowObserver> observation_{
+      this};
+};
+
+}  // namespace
+
+// Regression test for use-after-free in Close() (b/553134101).
+TEST_F(NativeWidgetAuraTest, CloseTailUseAfterFreeViaVisibilityObserver) {
+  auto widget = std::make_unique<Widget>();
+  Widget::InitParams params(Widget::InitParams::Ownership::CLIENT_OWNS_WIDGET,
+                            Widget::InitParams::TYPE_POPUP);
+  params.parent = root_window();
+  params.bounds = gfx::Rect(10, 10, 200, 200);
+  widget->Init(std::move(params));
+  widget->Show();
+
+  DeleteWindowOnHide deleter(widget->GetNativeView());
+  widget->Close();
+}
+
 }  // namespace views

diff --git a/ui/views/widget/native_widget_aura_unittest.cc b/ui/views/widget/native_widget_aura_unittest.cc
index 77c1de9..0ccbf8f 100644
--- a/ui/views/widget/native_widget_aura_unittest.cc
+++ b/ui/views/widget/native_widget_aura_unittest.cc
@@ -1223,4 +1223,42 @@
   native_widget_->UpdateVisualState();
 }
 
+namespace {
+
+// An aura::WindowObserver that deletes the window when it is hidden.
+class DeleteWindowOnHide : public aura::WindowObserver {
+ public:
+  explicit DeleteWindowOnHide(aura::Window* window) {
+    observation_.Observe(window);
+  }
+
+  // aura::WindowObserver:
+  void OnWindowVisibilityChanged(aura::Window* window, bool visible) override {
+    if (!visible && observation_.IsObservingSource(window)) {
+      observation_.Reset();
+      delete window;
+    }
+  }
+
+ private:
+  base::ScopedObservation<aura::Window, aura::WindowObserver> observation_{
+      this};
+};
+
+}  // namespace
+
+// Regression test for use-after-free in Close() (b/553134101).
+TEST_F(NativeWidgetAuraTest, CloseTailUseAfterFreeViaVisibilityObserver) {
+  auto widget = std::make_unique<Widget>();
+  Widget::InitParams params(Widget::InitParams::Ownership::CLIENT_OWNS_WIDGET,
+                            Widget::InitParams::TYPE_POPUP);
+  params.parent = root_window();
+  params.bounds = gfx::Rect(10, 10, 200, 200);
+  widget->Init(std::move(params));
+  widget->Show();
+
+  DeleteWindowOnHide deleter(widget->GetNativeView());
+  widget->Close();
+}
+
 }  // namespace views