From 11d511606e9d75ff9149f7b10fddd5e6af55af4a Mon Sep 17 00:00:00 2001
From: Justin Lulejian <[email protected]>
Date: Thu, 30 Jul 2026 13:05:24 -0700
Subject: [PATCH] [Extensions] Prevent UAF in EventEmitter::DispatchSync

Prior to this commit, an issue occurred during synchronous event
dispatch in EventEmitter::DispatchSync(). An event listener could throw
an exception where its custom stack getter invalidates and tears down
the v8::Context during exception handling
(ExceptionHandler::HandleException()). When this happened, the dispatch
loop continued without verifying if the context was still valid. As a
result, subsequent listeners in the loop attempted to execute using a
dangling, heap-freed JSRunner pointer, causing a renderer
heap-use-after-free read.

After this commit, EventEmitter::DispatchSync() checks whether the
v8::Context is still valid immediately after exception handling. If the
context was invalidated during error inspection, event dispatch safely
breaks out without attempting to dispatch to remaining listeners or
dereferencing freed per-context data. Any prior listener (if any) return
values are still returned to interested observers.

TAG=agy
CONV=7010bfb3-0b53-48c6-a45c-eefea11623ec

Fixed: 536676756
Change-Id: Ie4c03c7d65526f98f3b417b1d7a7367b89ae432b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8130654
Commit-Queue: Devlin Cronin <[email protected]>
Commit-Queue: Justin Lulejian <[email protected]>
Auto-Submit: Justin Lulejian <[email protected]>
Reviewed-by: Devlin Cronin <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1671354}
---
diff --git a/extensions/renderer/bindings/event_emitter.cc b/extensions/renderer/bindings/event_emitter.cc
index a0c9d2fc..f09040b 100644
--- a/extensions/renderer/bindings/event_emitter.cc
+++ b/extensions/renderer/bindings/event_emitter.cc
@@ -300,6 +300,16 @@
       }
       exception_handler_->HandleException(context, "Error in event handler",
                                           &try_catch);
+      // `ExceptionHandler:HandleException()` can run arbitrary JS code (e.g. if
+      // the thrown error defines a custom `stack` property getter), which might
+      // invalidate and tear down `context`. In this case, bail out to avoid
+      // executing remaining listeners on a destroyed context or dereferencing
+      // freed per-context data. We `break` so that any previous listeners
+      // return values are sent back to interested callers.
+      if (!binding::IsContextValid(context)) {
+        context.Clear();
+        break;
+      }
       try_catch.Reset();
     }
   }
diff --git a/chrome/browser/extensions/native_bindings_apitest.cc b/chrome/browser/extensions/native_bindings_apitest.cc
index 137e264d..dd40875 100644
--- a/chrome/browser/extensions/native_bindings_apitest.cc
+++ b/chrome/browser/extensions/native_bindings_apitest.cc
@@ -16,6 +16,7 @@
 #include "chrome/browser/renderer_context_menu/render_view_context_menu_test_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/test/base/ui_test_utils.h"
 #include "components/sessions/content/session_tab_helper.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/browser_test_utils.h"
@@ -29,6 +30,7 @@
 #include "extensions/browser/extension_util.h"
 #include "extensions/browser/process_manager.h"
 #include "extensions/browser/script_result_queue.h"
+#include "extensions/common/extension.h"
 #include "extensions/common/extension_features.h"
 #include "extensions/common/mojom/view_type.mojom.h"
 #include "extensions/common/switches.h"
@@ -36,6 +38,7 @@
 #include "extensions/test/result_catcher.h"
 #include "extensions/test/test_extension_dir.h"
 #include "net/dns/mock_host_resolver.h"
+#include "ui/base/window_open_disposition.h"
 
 namespace extensions {
 
@@ -1247,4 +1250,129 @@
   EXPECT_EQ("success", call_in_service_worker("verifyApiIsNotAvailable();"));
 }
 
+// Tests that when an event listener throws an exception whose `stack` getter
+// removes the `iframe` (destroying its context), synchronous event dispatch
+// safely breaks out without a heap-use-after-free on the freed
+// `extensions::JSRunner`, while ensuring a one-time message sender receives
+// the response sent by a previous listener before context teardown. This is a
+// regression test for `crbug.com/536676756` when run on AddressSanitizer
+// builders. It is not limited to run just on AddressSanitizer builders so we
+// can get code coverage too.
+IN_PROC_BROWSER_TEST_F(NativeBindingsApiTest,
+                       ListenersDestroyingFrameContextInErrorStackGetter) {
+  static constexpr char kManifest[] =
+      R"({
+           "name": "Events UAF Reproducer",
+           "version": "0.1",
+           "manifest_version": 3,
+           "permissions": ["storage"]
+         })";
+
+  static constexpr char kPageHtml[] =
+      R"(<!DOCTYPE html>
+         <html>
+         <head>
+           <script src="page.js"></script>
+         </head>
+         <body>
+           <iframe id="test_frame" src="frame.html"></iframe>
+         </body>
+         </html>)";
+
+  static constexpr char kPageJs[] =
+      R"('use strict';
+
+         window.onload = function() {
+           // Send a one-time message to trigger `chrome.runtime.onMessage`.
+           chrome.runtime.sendMessage('test_message', function(response) {
+             chrome.test.sendMessage('got_response: ' + response);
+           });
+         };)";
+
+  static constexpr char kFrameHtml[] =
+      R"(<!DOCTYPE html>
+         <html>
+         <head>
+           <script src="frame.js"></script>
+         </head>
+         <body></body>
+         </html>)";
+
+  static constexpr char kFrameJs[] =
+      R"('use strict';
+
+         // Add the first listener, which sends a reply to the sender via
+         // `sendResponse()` and also returns a value.
+         chrome.runtime.onMessage.addListener(
+             function firstListener(message, sender, sendResponse) {
+               sendResponse('first_listener_reply');
+               return 'first_listener_return_value';
+             });
+
+         // Add the second listener, which calls `sendResponse()` and then
+         // throws an error object with a custom `stack` property getter.
+         // When `extensions::ExceptionHandler::HandleException()` inspects the
+         // exception stack trace via `v8::TryCatch::StackTrace()`, the getter
+         // executes reentrantly and calls `frameElement.remove()`. That
+         // detaches the `iframe` from the DOM, destroys its `v8::Context`,
+         // and frees the per-context `extensions::JSRunner` data on the heap.
+         chrome.runtime.onMessage.addListener(
+             function secondListener(message, sender, sendResponse) {
+               sendResponse('second_listener_reply');
+               const exception = {};
+               Object.defineProperty(exception, 'stack', {
+                 get() {
+                   frameElement.remove();
+                 },
+               });
+               throw exception;
+             });
+
+         // Add a third listener. Previously,
+         // `extensions::EventEmitter::DispatchSync()` failed to check
+         // `extensions::binding::IsContextValid()` after exception handling
+         // and proceeded to execute this third listener using the dangling
+         // `extensions::JSRunner` pointer, triggering a heap-use-after-free
+         // read. `extensions::EventEmitter::DispatchSync()` now breaks the
+         // loop without calling this third listener.
+         chrome.runtime.onMessage.addListener(
+             function thirdListener(message, sender, sendResponse) {
+               sendResponse('third_listener_reply');
+             });)";
+
+  // Write the extension manifest, html, and js test files to the test
+  // directory.
+  TestExtensionDir test_dir;
+  test_dir.WriteManifest(kManifest);
+  test_dir.WriteFile(FILE_PATH_LITERAL("page.html"), kPageHtml);
+  test_dir.WriteFile(FILE_PATH_LITERAL("page.js"), kPageJs);
+  test_dir.WriteFile(FILE_PATH_LITERAL("frame.html"), kFrameHtml);
+  test_dir.WriteFile(FILE_PATH_LITERAL("frame.js"), kFrameJs);
+
+  // Set up a message listener to wait for the expected response from the
+  // first event listener.
+  ExtensionTestMessageListener response_listener(
+      /*expected_message=*/"got_response: first_listener_reply");
+
+  // Load the unpacked extension and assert that it loaded successfully.
+  const Extension* extension = LoadExtension(test_dir.UnpackedPath());
+  ASSERT_TRUE(extension);
+
+  // Navigate to `page.html`, which loads the iframe and triggers one-time
+  // messaging via `ui_test_utils::NavigateToURLWithDisposition()`.
+  ASSERT_TRUE(ui_test_utils::NavigateToURLWithDisposition(
+      browser(), extension->GetResourceURL("page.html"),
+      WindowOpenDisposition::NEW_FOREGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_LOAD_STOP));
+
+  // Verify that the one-time message sender received the reply from
+  // `firstListener` even though `secondListener` destroyed the iframe context.
+  {
+    SCOPED_TRACE(
+        "Waiting for the one-time message sender to receive the reply from "
+        "firstListener");
+    EXPECT_TRUE(response_listener.WaitUntilSatisfied());
+  }
+}
+
 }  // namespace extensions

diff --git a/chrome/browser/extensions/native_bindings_apitest.cc b/chrome/browser/extensions/native_bindings_apitest.cc
index 137e264d..dd40875 100644
--- a/chrome/browser/extensions/native_bindings_apitest.cc
+++ b/chrome/browser/extensions/native_bindings_apitest.cc
@@ -16,6 +16,7 @@
 #include "chrome/browser/renderer_context_menu/render_view_context_menu_test_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/test/base/ui_test_utils.h"
 #include "components/sessions/content/session_tab_helper.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/browser_test_utils.h"
@@ -29,6 +30,7 @@
 #include "extensions/browser/extension_util.h"
 #include "extensions/browser/process_manager.h"
 #include "extensions/browser/script_result_queue.h"
+#include "extensions/common/extension.h"
 #include "extensions/common/extension_features.h"
 #include "extensions/common/mojom/view_type.mojom.h"
 #include "extensions/common/switches.h"
@@ -36,6 +38,7 @@
 #include "extensions/test/result_catcher.h"
 #include "extensions/test/test_extension_dir.h"
 #include "net/dns/mock_host_resolver.h"
+#include "ui/base/window_open_disposition.h"
 
 namespace extensions {
 
@@ -1247,4 +1250,129 @@
   EXPECT_EQ("success", call_in_service_worker("verifyApiIsNotAvailable();"));
 }
 
+// Tests that when an event listener throws an exception whose `stack` getter
+// removes the `iframe` (destroying its context), synchronous event dispatch
+// safely breaks out without a heap-use-after-free on the freed
+// `extensions::JSRunner`, while ensuring a one-time message sender receives
+// the response sent by a previous listener before context teardown. This is a
+// regression test for `crbug.com/536676756` when run on AddressSanitizer
+// builders. It is not limited to run just on AddressSanitizer builders so we
+// can get code coverage too.
+IN_PROC_BROWSER_TEST_F(NativeBindingsApiTest,
+                       ListenersDestroyingFrameContextInErrorStackGetter) {
+  static constexpr char kManifest[] =
+      R"({
+           "name": "Events UAF Reproducer",
+           "version": "0.1",
+           "manifest_version": 3,
+           "permissions": ["storage"]
+         })";
+
+  static constexpr char kPageHtml[] =
+      R"(<!DOCTYPE html>
+         <html>
+         <head>
+           <script src="page.js"></script>
+         </head>
+         <body>
+           <iframe id="test_frame" src="frame.html"></iframe>
+         </body>
+         </html>)";
+
+  static constexpr char kPageJs[] =
+      R"('use strict';
+
+         window.onload = function() {
+           // Send a one-time message to trigger `chrome.runtime.onMessage`.
+           chrome.runtime.sendMessage('test_message', function(response) {
+             chrome.test.sendMessage('got_response: ' + response);
+           });
+         };)";
+
+  static constexpr char kFrameHtml[] =
+      R"(<!DOCTYPE html>
+         <html>
+         <head>
+           <script src="frame.js"></script>
+         </head>
+         <body></body>
+         </html>)";
+
+  static constexpr char kFrameJs[] =
+      R"('use strict';
+
+         // Add the first listener, which sends a reply to the sender via
+         // `sendResponse()` and also returns a value.
+         chrome.runtime.onMessage.addListener(
+             function firstListener(message, sender, sendResponse) {
+               sendResponse('first_listener_reply');
+               return 'first_listener_return_value';
+             });
+
+         // Add the second listener, which calls `sendResponse()` and then
+         // throws an error object with a custom `stack` property getter.
+         // When `extensions::ExceptionHandler::HandleException()` inspects the
+         // exception stack trace via `v8::TryCatch::StackTrace()`, the getter
+         // executes reentrantly and calls `frameElement.remove()`. That
+         // detaches the `iframe` from the DOM, destroys its `v8::Context`,
+         // and frees the per-context `extensions::JSRunner` data on the heap.
+         chrome.runtime.onMessage.addListener(
+             function secondListener(message, sender, sendResponse) {
+               sendResponse('second_listener_reply');
+               const exception = {};
+               Object.defineProperty(exception, 'stack', {
+                 get() {
+                   frameElement.remove();
+                 },
+               });
+               throw exception;
+             });
+
+         // Add a third listener. Previously,
+         // `extensions::EventEmitter::DispatchSync()` failed to check
+         // `extensions::binding::IsContextValid()` after exception handling
+         // and proceeded to execute this third listener using the dangling
+         // `extensions::JSRunner` pointer, triggering a heap-use-after-free
+         // read. `extensions::EventEmitter::DispatchSync()` now breaks the
+         // loop without calling this third listener.
+         chrome.runtime.onMessage.addListener(
+             function thirdListener(message, sender, sendResponse) {
+               sendResponse('third_listener_reply');
+             });)";
+
+  // Write the extension manifest, html, and js test files to the test
+  // directory.
+  TestExtensionDir test_dir;
+  test_dir.WriteManifest(kManifest);
+  test_dir.WriteFile(FILE_PATH_LITERAL("page.html"), kPageHtml);
+  test_dir.WriteFile(FILE_PATH_LITERAL("page.js"), kPageJs);
+  test_dir.WriteFile(FILE_PATH_LITERAL("frame.html"), kFrameHtml);
+  test_dir.WriteFile(FILE_PATH_LITERAL("frame.js"), kFrameJs);
+
+  // Set up a message listener to wait for the expected response from the
+  // first event listener.
+  ExtensionTestMessageListener response_listener(
+      /*expected_message=*/"got_response: first_listener_reply");
+
+  // Load the unpacked extension and assert that it loaded successfully.
+  const Extension* extension = LoadExtension(test_dir.UnpackedPath());
+  ASSERT_TRUE(extension);
+
+  // Navigate to `page.html`, which loads the iframe and triggers one-time
+  // messaging via `ui_test_utils::NavigateToURLWithDisposition()`.
+  ASSERT_TRUE(ui_test_utils::NavigateToURLWithDisposition(
+      browser(), extension->GetResourceURL("page.html"),
+      WindowOpenDisposition::NEW_FOREGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_LOAD_STOP));
+
+  // Verify that the one-time message sender received the reply from
+  // `firstListener` even though `secondListener` destroyed the iframe context.
+  {
+    SCOPED_TRACE(
+        "Waiting for the one-time message sender to receive the reply from "
+        "firstListener");
+    EXPECT_TRUE(response_listener.WaitUntilSatisfied());
+  }
+}
+
 }  // namespace extensions