From a66bf3aeca0d239b36e6e54487d91454d05bb486 Mon Sep 17 00:00:00 2001
From: Jonathan Ross <jonross@chromium.org>
Date: Thu, 07 May 2026 11:56:41 -0700
Subject: [PATCH] Fix Use-After-Free in RenderWidgetHostViewAura::OnBoundsChanged

On Windows, RenderWidgetHostViewAura::OnBoundsChanged() calls
GetInputMethod()->OnCaretBoundsChanged(this), which can trigger a
synchronous COM callout to a third-party TSF IME. If the IME pumps the
thread message queue, a pending task (such as a popup close IPC) can
synchronously destroy the view while OnBoundsChanged() is still on the
stack.

This leads to a Use-After-Free when UpdateInsetsWithVirtualKeyboardEnabled()
is called after the IME callout returns, and a Write-After-Free when the
stack-allocated base::AutoReset<bool> destructor attempts to reset
in_bounds_changed_ in the freed object.

This CL fixes the issue by:
1. Replacing base::AutoReset with base::WeakAutoReset to safely handle
   object destruction during the reset scope.
2. Adding a WeakPtr check after the IME callout to skip further member
   access if the object was destroyed.

Bug: 502112506
Change-Id: Id64cc95569c98a4c62318b7f86e11d34365efdfe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7819560
Reviewed-by: Aman Verma <amanvr@google.com>
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1627112}
---

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 b6f9361..9903327 100644
--- a/content/browser/renderer_host/render_widget_host_view_aura.cc
+++ b/content/browser/renderer_host/render_widget_host_view_aura.cc
@@ -16,6 +16,7 @@
 #include "base/functional/callback_helpers.h"
 #include "base/logging.h"
 #include "base/memory/raw_ptr.h"
+#include "base/memory/weak_auto_reset.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/notimplemented.h"
 #include "base/strings/strcat.h"
@@ -2235,7 +2236,13 @@
 
 void RenderWidgetHostViewAura::OnBoundsChanged(const gfx::Rect& old_bounds,
                                                const gfx::Rect& new_bounds) {
-  base::AutoReset<bool> in_bounds_changed(&in_bounds_changed_, true);
+  // OnCaretBoundsChanged() below may call out to a third-party TSF IME on
+  // Windows, which can re-entrantly destroy `this`. Use WeakAutoReset so the
+  // unwind write does not land in freed memory (AutoReset::scoped_variable_ is
+  // RAW_PTR_EXCLUSION and not BRP-protected).
+  base::WeakAutoReset in_bounds_changed(
+      weak_ptr_factory_.GetWeakPtr(),
+      &RenderWidgetHostViewAura::in_bounds_changed_, true);
   // We care about this whenever RenderWidgetHostViewAura is not owned by a
   // WebContentsViewAura since changes to the Window's bounds need to be
   // messaged to the renderer.  WebContentsViewAura invokes SetSize() or
@@ -2244,7 +2251,12 @@
   SetSize(new_bounds.size());
 
   if (GetInputMethod()) {
+    auto weak_this = weak_ptr_factory_.GetWeakPtr();
     GetInputMethod()->OnCaretBoundsChanged(this);
+    // `this` may have been deleted inside the IME callout.
+    if (!weak_this) {
+      return;
+    }
     UpdateInsetsWithVirtualKeyboardEnabled();
   }
 }
diff --git a/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc b/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
index 0a69d55..feb608c4 100644
--- a/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
+++ b/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
@@ -6861,6 +6861,84 @@
   GetInputMethod()->RemoveObserver(this);
 }
 
+// Mock InputMethod that runs a closure when OnCaretBoundsChanged is invoked.
+// Simulates a Windows TSF IME whose ITextStoreACPSink::OnLayoutChange handler
+// pumps the thread message queue, allowing a queued task to synchronously
+// destroy the RenderWidgetHostViewAura while it is still inside
+// OnBoundsChanged().
+class DestroyingMockInputMethod : public ui::MockInputMethod {
+ public:
+  DestroyingMockInputMethod() : ui::MockInputMethod(nullptr) {}
+
+  void OnCaretBoundsChanged(const ui::TextInputClient* client) override {
+    ui::MockInputMethod::OnCaretBoundsChanged(client);
+    if (on_caret_bounds_changed_) {
+      std::move(on_caret_bounds_changed_).Run();
+    }
+  }
+
+  void set_on_caret_bounds_changed(base::OnceClosure closure) {
+    on_caret_bounds_changed_ = std::move(closure);
+  }
+
+ private:
+  base::OnceClosure on_caret_bounds_changed_;
+};
+
+class RenderWidgetHostViewAuraOnBoundsChangedUAFTest
+    : public RenderWidgetHostViewAuraTest {
+ public:
+  void SetUp() override {
+    input_method_ = new DestroyingMockInputMethod();
+    // Ownership is transferred to the WindowTreeHost via the InputMethod
+    // factory; see SetUpInputMethodForTesting().
+    ui::SetUpInputMethodForTesting(input_method_);
+    SetUpEnvironment();
+  }
+
+  void TearDown() override {
+    input_method_ = nullptr;
+    RenderWidgetHostViewAuraTest::TearDown();
+  }
+
+ protected:
+  raw_ptr<DestroyingMockInputMethod> input_method_ = nullptr;
+};
+
+// RWHVA::OnBoundsChanged() constructs a base::AutoReset<bool> holding
+// &in_bounds_changed_, then calls GetInputMethod()->OnCaretBoundsChanged(this).
+// On Windows that reaches TSFTextStore::SendOnLayoutChange ->
+// text_store_acp_sink_->OnLayoutChange(), a synchronous COM call into the
+// active third-party IME. If the IME pumps messages and the view is destroyed
+// re-entrantly, on unwind UpdateInsetsWithVirtualKeyboardEnabled() and
+// ~AutoReset both touch freed memory. AutoReset::scoped_variable_ is
+// RAW_PTR_EXCLUSION, so it is not MiraclePtr-protected: the ~AutoReset write
+// lands in a freed (un-quarantined) slot.
+TEST_F(RenderWidgetHostViewAuraOnBoundsChangedUAFTest,
+       DestroyDuringOnCaretBoundsChanged) {
+  InitViewForFrame(nullptr);
+  ParentHostView(view_, parent_view_);
+  // `view_` shares the root window (and thus the InputMethod) with
+  // `parent_view_`.
+  ASSERT_EQ(static_cast<ui::InputMethod*>(input_method_.get()),
+            GetInputMethod());
+
+  // Arrange for the view to be synchronously destroyed inside
+  // OnCaretBoundsChanged, simulating re-entrant destruction triggered by a
+  // TSF IME callout that pumps a queued window.close() / renderer-gone task.
+  FakeRenderWidgetHostViewAura* raw_view = view_.get();
+  input_method_->set_on_caret_bounds_changed(base::BindLambdaForTesting([&]() {
+    widget_host_ = nullptr;
+    view_.ExtractAsDangling()->Destroy();
+  }));
+
+  // Under ASAN this triggers heap-use-after-free in
+  // UpdateInsetsWithVirtualKeyboardEnabled() (read of freed
+  // keyboard_occluded_bounds_) followed by a write-after-free in
+  // ~AutoReset<bool> to the freed in_bounds_changed_ slot.
+  raw_view->OnBoundsChanged(gfx::Rect(), gfx::Rect(0, 0, 100, 100));
+}
+
 #if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_CHROMEOS)
 class MockVirtualKeyboardController final
     : public ui::VirtualKeyboardController {

diff --git a/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc b/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
index 0a69d55..feb608c4 100644
--- a/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
+++ b/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
@@ -6861,6 +6861,84 @@
   GetInputMethod()->RemoveObserver(this);
 }
 
+// Mock InputMethod that runs a closure when OnCaretBoundsChanged is invoked.
+// Simulates a Windows TSF IME whose ITextStoreACPSink::OnLayoutChange handler
+// pumps the thread message queue, allowing a queued task to synchronously
+// destroy the RenderWidgetHostViewAura while it is still inside
+// OnBoundsChanged().
+class DestroyingMockInputMethod : public ui::MockInputMethod {
+ public:
+  DestroyingMockInputMethod() : ui::MockInputMethod(nullptr) {}
+
+  void OnCaretBoundsChanged(const ui::TextInputClient* client) override {
+    ui::MockInputMethod::OnCaretBoundsChanged(client);
+    if (on_caret_bounds_changed_) {
+      std::move(on_caret_bounds_changed_).Run();
+    }
+  }
+
+  void set_on_caret_bounds_changed(base::OnceClosure closure) {
+    on_caret_bounds_changed_ = std::move(closure);
+  }
+
+ private:
+  base::OnceClosure on_caret_bounds_changed_;
+};
+
+class RenderWidgetHostViewAuraOnBoundsChangedUAFTest
+    : public RenderWidgetHostViewAuraTest {
+ public:
+  void SetUp() override {
+    input_method_ = new DestroyingMockInputMethod();
+    // Ownership is transferred to the WindowTreeHost via the InputMethod
+    // factory; see SetUpInputMethodForTesting().
+    ui::SetUpInputMethodForTesting(input_method_);
+    SetUpEnvironment();
+  }
+
+  void TearDown() override {
+    input_method_ = nullptr;
+    RenderWidgetHostViewAuraTest::TearDown();
+  }
+
+ protected:
+  raw_ptr<DestroyingMockInputMethod> input_method_ = nullptr;
+};
+
+// RWHVA::OnBoundsChanged() constructs a base::AutoReset<bool> holding
+// &in_bounds_changed_, then calls GetInputMethod()->OnCaretBoundsChanged(this).
+// On Windows that reaches TSFTextStore::SendOnLayoutChange ->
+// text_store_acp_sink_->OnLayoutChange(), a synchronous COM call into the
+// active third-party IME. If the IME pumps messages and the view is destroyed
+// re-entrantly, on unwind UpdateInsetsWithVirtualKeyboardEnabled() and
+// ~AutoReset both touch freed memory. AutoReset::scoped_variable_ is
+// RAW_PTR_EXCLUSION, so it is not MiraclePtr-protected: the ~AutoReset write
+// lands in a freed (un-quarantined) slot.
+TEST_F(RenderWidgetHostViewAuraOnBoundsChangedUAFTest,
+       DestroyDuringOnCaretBoundsChanged) {
+  InitViewForFrame(nullptr);
+  ParentHostView(view_, parent_view_);
+  // `view_` shares the root window (and thus the InputMethod) with
+  // `parent_view_`.
+  ASSERT_EQ(static_cast<ui::InputMethod*>(input_method_.get()),
+            GetInputMethod());
+
+  // Arrange for the view to be synchronously destroyed inside
+  // OnCaretBoundsChanged, simulating re-entrant destruction triggered by a
+  // TSF IME callout that pumps a queued window.close() / renderer-gone task.
+  FakeRenderWidgetHostViewAura* raw_view = view_.get();
+  input_method_->set_on_caret_bounds_changed(base::BindLambdaForTesting([&]() {
+    widget_host_ = nullptr;
+    view_.ExtractAsDangling()->Destroy();
+  }));
+
+  // Under ASAN this triggers heap-use-after-free in
+  // UpdateInsetsWithVirtualKeyboardEnabled() (read of freed
+  // keyboard_occluded_bounds_) followed by a write-after-free in
+  // ~AutoReset<bool> to the freed in_bounds_changed_ slot.
+  raw_view->OnBoundsChanged(gfx::Rect(), gfx::Rect(0, 0, 100, 100));
+}
+
 #if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_CHROMEOS)
 class MockVirtualKeyboardController final
     : public ui::VirtualKeyboardController {