From dadcd96cf1f88f544995631521471cf64b90a7ff Mon Sep 17 00:00:00 2001
From: Tom Anderson <[email protected]>
Date: Wed, 27 May 2026 17:37:03 -0700
Subject: [PATCH] [Ozone/Wayland] Fix UAF in HandleToplevelConfigure via OnActivationChanged

During HandleToplevelConfigure, calling
HandleToplevelConfigureWithOrigin can trigger OnActivationChanged on the
delegate. Since views/widget layer handling can synchronously destroy
the widget and thereby the platform window itself, calling subsequent
functions on `this` results in a Use-After-Free (UAF).

This CL adds WeakPtr guards to both HandleToplevelConfigure and
HandleToplevelConfigureWithOrigin to prevent referencing the object
after it has been synchronously closed.

Fixed: 517046249
Change-Id: Ic7d3ac624033af9fc07d7f73a98e4307e803492d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7880866
Commit-Queue: Thomas Anderson <[email protected]>
Commit-Queue: Thomas Lukaszewicz <[email protected]>
Auto-Submit: Thomas Anderson <[email protected]>
Reviewed-by: Thomas Lukaszewicz <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1637352}
---

diff --git a/ui/ozone/platform/wayland/host/wayland_toplevel_window.cc b/ui/ozone/platform/wayland/host/wayland_toplevel_window.cc
index 6c3e6a341..fe83d31 100644
--- a/ui/ozone/platform/wayland/host/wayland_toplevel_window.cc
+++ b/ui/ozone/platform/wayland/host/wayland_toplevel_window.cc
@@ -465,7 +465,15 @@
     int32_t width_dip,
     int32_t height_dip,
     const WindowStates& window_states) {
+  // HandleToplevelConfigureWithOrigin() calls into the delegate
+  // (OnActivationChanged et al.), which the views layer documents may
+  // synchronously close the widget and destroy this platform window. See
+  // DesktopWindowTreeHostPlatform::OnActivationChanged().
+  auto alive = weak_ptr_factory_.GetWeakPtr();
   HandleToplevelConfigureWithOrigin(0, 0, width_dip, height_dip, window_states);
+  if (!alive) {
+    return;
+  }
   UpdateSessionStateIfNeeded();
 }
 
@@ -566,7 +574,11 @@
     SetRestoredBoundsInDIP(GetBoundsInDIP());
   }
 
+  auto alive = weak_ptr_factory_.GetWeakPtr();
   UpdateActivationState();
+  if (!alive) {
+    return;
+  }
   if (prev_suspended != is_suspended_) {
     frame_manager()->OnWindowSuspensionChanged();
   }
diff --git a/ui/ozone/platform/wayland/host/wayland_window_unittest.cc b/ui/ozone/platform/wayland/host/wayland_window_unittest.cc
index d0afdc8..7aea341 100644
--- a/ui/ozone/platform/wayland/host/wayland_window_unittest.cc
+++ b/ui/ozone/platform/wayland/host/wayland_window_unittest.cc
@@ -5428,6 +5428,45 @@
   EXPECT_EQ(window_->applied_state(), previous_state);
 }
 
+// Regression POC: WaylandToplevelWindow::HandleToplevelConfigure() continues to
+// use `this` after delegate()->OnActivationChanged() synchronously destroys the
+// platform window. This mirrors the production path documented at
+// DesktopWindowTreeHostPlatform::OnActivationChanged where
+// HandleActivationChanged() can synchronously close the widget, which in turn
+// calls SetPlatformWindow(nullptr) and frees the WaylandToplevelWindow while
+// the xdg_toplevel.configure handler is still on the stack.
+TEST_P(WaylandWindowTest, HandleToplevelConfigureSyncCloseOnDeactivate) {
+  // After SetUp(), |window_| has already received an activated configure, so
+  // is_xdg_active_ == is_active_ == true.
+  ASSERT_TRUE(window_);
+  WaylandWindow* raw_window = window_.get();
+
+  // Simulate a delegate that destroys the platform window inside
+  // OnActivationChanged(false) — exactly what happens in production when a
+  // WidgetObserver calls Widget::CloseNow() on deactivation, leading to
+  // DesktopWindowTreeHostPlatform::OnClosed -> SetPlatformWindow(nullptr).
+  EXPECT_CALL(delegate_, OnActivationChanged(Eq(false)))
+      .WillOnce(InvokeWithoutArgs([this]() { window_.reset(); }));
+
+  // Don't try to talk to the server after the window has been torn down
+  // mid-dispatch.
+  DisableSyncOnTearDown();
+
+  // Drive the standard xdg_toplevel.configure entry point with the activated
+  // bit cleared. This calls HandleToplevelConfigureWithOrigin() ->
+  // UpdateActivationState() -> delegate()->OnActivationChanged(false), which
+  // (via the mock above) frees `this`. Control then returns to
+  // HandleToplevelConfigure:469 which calls UpdateSessionStateIfNeeded() on
+  // the freed object.
+  WaylandWindow::WindowStates deactivated_states;
+  deactivated_states.is_activated = false;
+  raw_window->HandleToplevelConfigure(800, 600, deactivated_states);
+
+  // If we got here without ASAN reporting a heap-use-after-free, the bug is
+  // fixed.
+  EXPECT_FALSE(window_);
+}
+
 INSTANTIATE_TEST_SUITE_P(XdgVersionStableTest,
                          WaylandWindowTest,
                          Values(wl::ServerConfig{}));

diff --git a/ui/ozone/platform/wayland/host/wayland_window_unittest.cc b/ui/ozone/platform/wayland/host/wayland_window_unittest.cc
index d0afdc8..7aea341 100644
--- a/ui/ozone/platform/wayland/host/wayland_window_unittest.cc
+++ b/ui/ozone/platform/wayland/host/wayland_window_unittest.cc
@@ -5428,6 +5428,45 @@
   EXPECT_EQ(window_->applied_state(), previous_state);
 }
 
+// Regression POC: WaylandToplevelWindow::HandleToplevelConfigure() continues to
+// use `this` after delegate()->OnActivationChanged() synchronously destroys the
+// platform window. This mirrors the production path documented at
+// DesktopWindowTreeHostPlatform::OnActivationChanged where
+// HandleActivationChanged() can synchronously close the widget, which in turn
+// calls SetPlatformWindow(nullptr) and frees the WaylandToplevelWindow while
+// the xdg_toplevel.configure handler is still on the stack.
+TEST_P(WaylandWindowTest, HandleToplevelConfigureSyncCloseOnDeactivate) {
+  // After SetUp(), |window_| has already received an activated configure, so
+  // is_xdg_active_ == is_active_ == true.
+  ASSERT_TRUE(window_);
+  WaylandWindow* raw_window = window_.get();
+
+  // Simulate a delegate that destroys the platform window inside
+  // OnActivationChanged(false) — exactly what happens in production when a
+  // WidgetObserver calls Widget::CloseNow() on deactivation, leading to
+  // DesktopWindowTreeHostPlatform::OnClosed -> SetPlatformWindow(nullptr).
+  EXPECT_CALL(delegate_, OnActivationChanged(Eq(false)))
+      .WillOnce(InvokeWithoutArgs([this]() { window_.reset(); }));
+
+  // Don't try to talk to the server after the window has been torn down
+  // mid-dispatch.
+  DisableSyncOnTearDown();
+
+  // Drive the standard xdg_toplevel.configure entry point with the activated
+  // bit cleared. This calls HandleToplevelConfigureWithOrigin() ->
+  // UpdateActivationState() -> delegate()->OnActivationChanged(false), which
+  // (via the mock above) frees `this`. Control then returns to
+  // HandleToplevelConfigure:469 which calls UpdateSessionStateIfNeeded() on
+  // the freed object.
+  WaylandWindow::WindowStates deactivated_states;
+  deactivated_states.is_activated = false;
+  raw_window->HandleToplevelConfigure(800, 600, deactivated_states);
+
+  // If we got here without ASAN reporting a heap-use-after-free, the bug is
+  // fixed.
+  EXPECT_FALSE(window_);
+}
+
 INSTANTIATE_TEST_SUITE_P(XdgVersionStableTest,
                          WaylandWindowTest,
                          Values(wl::ServerConfig{}));