From d4a5fe4028c21e514138d79b7f0b07beb5abc6fa Mon Sep 17 00:00:00 2001
From: Alex Moshchuk <[email protected]>
Date: Fri, 15 May 2026 10:38:34 -0700
Subject: [PATCH] Guard against UAF in LegacyRenderWidgetHostHWND::SetBounds()

LegacyRenderWidgetHostHWND::SetBounds() calls the ::SetWindowPos() Windows
API which can trigger a nested message loop and theoretically lead to
the destruction of RenderWidgetHostViewAura/LegacyRenderWidgetHostHWND.
Add some bandaid WeakPtr checks to guard against this.

Fixed: 513235131
Change-Id: Ia95e4bdbe1faf8ffe5e2a0956670005dff1d4841
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7848426
Reviewed-by: Andrew Paseltiner <[email protected]>
Commit-Queue: Alex Moshchuk <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1631398}
---

diff --git a/content/browser/renderer_host/legacy_render_widget_host_win.cc b/content/browser/renderer_host/legacy_render_widget_host_win.cc
index 27667c8e..5128325 100644
--- a/content/browser/renderer_host/legacy_render_widget_host_win.cc
+++ b/content/browser/renderer_host/legacy_render_widget_host_win.cc
@@ -196,9 +196,16 @@
 void LegacyRenderWidgetHostHWND::SetBounds(const gfx::Rect& bounds) {
   gfx::Rect bounds_in_pixel =
       display::win::GetScreenWin()->DIPToClientRect(hwnd(), bounds);
+  // ::SetWindowPos can spin a nested message loop, which can potentially
+  // destroy `this`.
+  base::WeakPtr<LegacyRenderWidgetHostHWND> ref(
+      msg_handler_weak_factory_.GetWeakPtr());
   ::SetWindowPos(hwnd(), nullptr, bounds_in_pixel.x(), bounds_in_pixel.y(),
                  bounds_in_pixel.width(), bounds_in_pixel.height(),
                  SWP_NOREDRAW);
+  if (!ref) {
+    return;
+  }
   if (direct_manipulation_helper_) {
     direct_manipulation_helper_->SetSizeInPixels(bounds_in_pixel.size());
   }
diff --git a/content/browser/renderer_host/render_widget_host_view_aura.cc b/content/browser/renderer_host/render_widget_host_view_aura.cc
index 3f497e5..6abce33 100644
--- a/content/browser/renderer_host/render_widget_host_view_aura.cc
+++ b/content/browser/renderer_host/render_widget_host_view_aura.cc
@@ -580,8 +580,15 @@
 void RenderWidgetHostViewAura::HandleBoundsInRootChanged() {
 #if BUILDFLAG(IS_WIN)
   if (legacy_render_widget_host_HWND_) {
+    // `SetBounds()` calls ::SetWindowPos which can spin a nested message loop
+    // on Windows, potentially destroying `this`.
+    base::WeakPtr<RenderWidgetHostViewAura> weak_this(
+        weak_ptr_factory_.GetWeakPtr());
     legacy_render_widget_host_HWND_->SetBounds(
         window_->GetBoundsInRootWindow());
+    if (!weak_this) {
+      return;
+    }
   }
 #endif
   if (!in_shutdown_) {
@@ -705,6 +712,8 @@
 
 #if BUILDFLAG(IS_WIN)
   UpdateLegacyWin();
+  // WARNING: Do not add any code after this line, since the last call can
+  // potentially destroy `this`.
 #endif
 }
 
@@ -3124,7 +3133,14 @@
                               window_->GetLocalSurfaceId());
 
 #if BUILDFLAG(IS_WIN)
+  // `UpdateLegacyWin()` can spin a nested message loop on Windows, potentially
+  // destroying `this`.
+  base::WeakPtr<RenderWidgetHostViewAura> weak_this(
+      weak_ptr_factory_.GetWeakPtr());
   UpdateLegacyWin();
+  if (!weak_this) {
+    return;
+  }
 
   if (IsPointerLocked()) {
     UpdateMouseLockRegion();
@@ -3154,9 +3170,19 @@
   }
 
   if (legacy_render_widget_host_HWND_) {
+    // Both UpdateParent and SetBounds can spin a nested message loop on
+    // Windows, potentially destroying `this`.
+    base::WeakPtr<RenderWidgetHostViewAura> weak_this(
+        weak_ptr_factory_.GetWeakPtr());
     legacy_render_widget_host_HWND_->UpdateParent(GetHostWindowHWND());
+    if (!weak_this) {
+      return;
+    }
     legacy_render_widget_host_HWND_->SetBounds(
         window_->GetBoundsInRootWindow());
+    if (!weak_this) {
+      return;
+    }
     // There are cases where the parent window is created, made visible and
     // the associated RenderWidget is also visible before the
     // LegacyRenderWidgetHostHWND instace is created. Ensure that it is shown
@@ -3187,7 +3213,14 @@
   }
 
 #if BUILDFLAG(IS_WIN)
+  // `UpdateLegacyWin()` can spin a nested message loop on Windows, potentially
+  // destroying `this`.
+  base::WeakPtr<RenderWidgetHostViewAura> weak_this(
+      weak_ptr_factory_.GetWeakPtr());
   UpdateLegacyWin();
+  if (!weak_this) {
+    return;
+  }
 #endif
 
   delegated_frame_host_->AttachToCompositor(GetCompositor());