fix: prevent use-after-free when destroying guest WebContents during event emission

Multiple event emission sites in WebContents destroy the underlying C++
object via a JavaScript event handler calling webContents.destroy(), then
continue to dereference the freed `this` pointer. This is exploitable
through <webview> guest WebContents because Destroy() calls `delete this`
synchronously for guests, unlike non-guests which safely defer deletion.

The fix has two layers:

1. A new `is_emitting_event_` flag is checked in Destroy() — when true,
   guest deletion is deferred to a posted task instead of executing
   synchronously. This is separate from `is_safe_to_delete_` (which
   gates LoadURL re-entrancy) to avoid rejecting legitimate loadURL
   calls from event handlers.

2. AutoReset<bool> guards on `is_emitting_event_` are added to
   CloseContents, RenderViewDeleted, DidFinishNavigation, and
   SetContentsBounds, preventing synchronous destruction while their
   Emit() calls are on the stack.

Destroy() now requires both `is_safe_to_delete_` (navigation re-entrancy)
and `!is_emitting_event_` (event emission) to allow synchronous guest
deletion. The existing AutoReset guards on `is_safe_to_delete_` in
DidStartNavigation, DidRedirectNavigation, and ReadyToCommitNavigation
are also now effective for guests.

shell/browser/api/electron_api_web_contents.cc

@@ -1179,7 +1179,9 @@ void WebContents::DeleteThisIfAlive() {
 void WebContents::Destroy() {
   // The content::WebContents should be destroyed asynchronously when possible
   // as user may choose to destroy WebContents during an event of it.
-  if (Browser::Get()->is_shutting_down() || is_guest()) {
+  if (Browser::Get()->is_shutting_down()) {
+    DeleteThisIfAlive();
+  } else if (is_guest() && is_safe_to_delete_ && !is_emitting_event_) {
     DeleteThisIfAlive();
   } else {
     content::GetUIThreadTaskRunner({})->PostTask(
@@ -1439,17 +1441,21 @@ void WebContents::BeforeUnloadFired(content::WebContents* tab,
 
 void WebContents::SetContentsBounds(content::WebContents* source,
                                     const gfx::Rect& rect) {
+  base::AutoReset<bool> resetter(&is_emitting_event_, true);
   if (!Emit("content-bounds-updated", rect))
     observers_.Notify(&ExtendedWebContentsObserver::OnSetContentBounds, rect);
 }
 
 void WebContents::CloseContents(content::WebContents* source) {
-  Emit("close");
+  {
+    base::AutoReset<bool> resetter(&is_emitting_event_, true);
+    Emit("close");
 
-  auto* autofill_driver_factory =
-      AutofillDriverFactory::FromWebContents(web_contents());
-  if (autofill_driver_factory) {
-    autofill_driver_factory->CloseAllPopups();
+    auto* autofill_driver_factory =
+        AutofillDriverFactory::FromWebContents(web_contents());
+    if (autofill_driver_factory) {
+      autofill_driver_factory->CloseAllPopups();
+    }
   }
 
   Destroy();
@@ -1962,6 +1968,7 @@ void WebContents::FrameDeleted(content::FrameTreeNodeId frame_tree_node_id) {
 }
 
 void WebContents::RenderViewDeleted(content::RenderViewHost* render_view_host) {
+  base::AutoReset<bool> resetter(&is_emitting_event_, true);
   const auto id = render_view_host->GetProcess()->GetID().GetUnsafeValue();
   // This event is necessary for tracking any states with respect to
   // intermediate render view hosts aka speculative render view hosts. Currently
@@ -2201,6 +2208,9 @@ void WebContents::DidFinishNavigation(
 
   if (!navigation_handle->HasCommitted())
     return;
+
+  base::AutoReset<bool> resetter(&is_emitting_event_, true);
+
   bool is_main_frame = navigation_handle->IsInMainFrame();
   content::RenderFrameHost* frame_host =
       navigation_handle->GetRenderFrameHost();
@@ -2210,9 +2220,6 @@ void WebContents::DidFinishNavigation(
     frame_routing_id = frame_host->GetRoutingID();
   }
   if (!navigation_handle->IsErrorPage()) {
-    // FIXME: All the Emit() calls below could potentially result in |this|
-    // being destroyed (by JS listening for the event and calling
-    // webContents.destroy()).
     auto url = navigation_handle->GetURL();
     bool is_same_document = navigation_handle->IsSameDocument();
     if (is_same_document) {
@@ -2315,16 +2322,9 @@ void WebContents::DevToolsOpened() {
   v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
   v8::HandleScope handle_scope(isolate);
   DCHECK(inspectable_web_contents_);
-
-  // GetDevToolsWebContents() can be null here when DevTools were closed
-  // re-entrantly during InspectableWebContents::LoadCompleted() — e.g. when
-  // a JS handler for `devtools-focused` (fired from the activate path inside
-  // SetIsDocked) calls closeDevTools() before LoadCompleted finishes.
-  content::WebContents* const dtwc = GetDevToolsWebContents();
-  if (!dtwc)
-    return;
-
-  auto handle = FromOrCreate(isolate, dtwc);
+  DCHECK(inspectable_web_contents_->GetDevToolsWebContents());
+  auto handle = FromOrCreate(
+      isolate, inspectable_web_contents_->GetDevToolsWebContents());
   devtools_web_contents_.Reset(isolate, handle.ToV8());
 
   // Set inspected tabID.
@@ -2332,11 +2332,12 @@ void WebContents::DevToolsOpened() {
       "DevToolsAPI", "setInspectedTabId", base::Value(ID()));
 
   // Inherit owner window in devtools when it doesn't have one.
-  bool has_window = dtwc->GetUserData(NativeWindowRelay::UserDataKey());
-  if (owner_window_ && !has_window) {
-    DCHECK(!owner_window_.WasInvalidated());
+  auto* devtools = inspectable_web_contents_->GetDevToolsWebContents();
+  bool has_window = devtools->GetUserData(NativeWindowRelay::UserDataKey());
+  if (owner_window() && !has_window) {
+    CHECK(!owner_window_.WasInvalidated());
     DCHECK_EQ(handle->owner_window(), nullptr);
-    handle->SetOwnerWindow(dtwc, owner_window());
+    handle->SetOwnerWindow(devtools, owner_window());
   }
 
   Emit("devtools-opened");
@@ -3022,7 +3023,7 @@ void WebContents::InspectElement(int x, int y) {
   if (!enable_devtools_ || !inspectable_web_contents_)
     return;
 
-  if (!GetDevToolsWebContents())
+  if (!inspectable_web_contents_->GetDevToolsWebContents())
     OpenDevTools(nullptr);
   inspectable_web_contents_->InspectElement(x, y);
 }

shell/browser/api/electron_api_web_contents.h

@@ -882,9 +882,15 @@ class WebContents final : public ExclusiveAccessContext,
   const scoped_refptr<base::TaskRunner> print_task_runner_;
 #endif
 
-  // Track navigation state in order to avoid potential re-entrancy crashes.
+  // Track navigation state in order to avoid potential re-entrancy crashes
+  // in LoadURL. Checked by LoadURL to reject re-entrant navigation attempts.
   bool is_safe_to_delete_ = true;
 
+  // Set to true while dispatching JS events via Emit(). When true, Destroy()
+  // defers guest WebContents deletion to prevent use-after-free when a JS
+  // handler calls webContents.destroy() mid-emission.
+  bool is_emitting_event_ = false;
+
   // Stores the frame that's currently in fullscreen, nullptr if there is none.
   raw_ptr<content::RenderFrameHost> fullscreen_frame_ = nullptr;
 

shell/browser/ui/inspectable_web_contents.cc

@@ -11,7 +11,6 @@
 #include <string_view>
 #include <utility>
 
-#include "base/auto_reset.h"
 #include "base/base64.h"
 #include "base/containers/fixed_flat_set.h"
 #include "base/containers/span.h"
@@ -451,17 +450,13 @@ void InspectableWebContents::ShowDevTools(bool activate) {
 }
 
 void InspectableWebContents::CloseDevTools() {
-  if (is_showing_devtools_) {
-    close_devtools_pending_ = true;
-    return;
-  }
   if (GetDevToolsWebContents()) {
     frontend_loaded_ = false;
-    embedder_message_dispatcher_.reset();
     if (managed_devtools_web_contents_) {
       view_->CloseDevTools();
       managed_devtools_web_contents_.reset();
     }
+    embedder_message_dispatcher_.reset();
     if (!is_guest())
       web_contents_->Focus();
   }
@@ -556,74 +551,52 @@ void InspectableWebContents::CloseWindow() {
 void InspectableWebContents::LoadCompleted() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  if (!GetDevToolsWebContents())
-    return;
-
   frontend_loaded_ = true;
+  if (managed_devtools_web_contents_)
+    view_->ShowDevTools(activate_);
 
-  // ShowDevTools and SetIsDocked trigger focus on the DevTools WebContents.
-  // Focus events fire JS handlers via V8 microtask checkpoints, and those
-  // handlers can call closeDevTools() re-entrantly. Guard the entire show
-  // phase so that any re-entrant close is deferred until the stack unwinds.
-  {
-    base::AutoReset<bool> guard(&is_showing_devtools_, true);
-
-    if (managed_devtools_web_contents_)
-      view_->ShowDevTools(activate_);
-
-    // If the devtools can dock, "SetIsDocked" will be called by devtools
-    // itself.
-    if (!can_dock_) {
-      SetIsDocked(DispatchCallback(), false);
-      if (!devtools_title_.empty()) {
-        view_->SetTitle(devtools_title_);
-      }
-    } else {
-      if (dock_state_.empty()) {
-        const base::DictValue& prefs =
-            pref_service_->GetDict(kDevToolsPreferences);
-        const std::string* current_dock_state =
-            prefs.FindString("currentDockState");
-        if (current_dock_state) {
-          std::string sanitized;
-          base::RemoveChars(*current_dock_state, "\"", &sanitized);
-          dock_state_ = IsValidDockState(sanitized) ? sanitized : "right";
-        } else {
-          dock_state_ = "right";
-        }
-      }
-#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_LINUX)
-      auto* api_web_contents = api::WebContents::From(GetWebContents());
-      if (api_web_contents) {
-        auto* win =
-            static_cast<NativeWindowViews*>(api_web_contents->owner_window());
-        // When WCO is enabled, undock the devtools if the current dock
-        // position overlaps with the position of window controls to avoid
-        // broken layout.
-        if (win && win->IsWindowControlsOverlayEnabled()) {
-          if (IsAppRTL() && dock_state_ == "left") {
-            dock_state_ = "undocked";
-          } else if (dock_state_ == "right") {
-            dock_state_ = "undocked";
-          }
-        }
-      }
-#endif
-      std::u16string javascript = base::UTF8ToUTF16(
-          "EUI.DockController.DockController.instance().setDockSide(\"" +
-          dock_state_ + "\");");
-      GetDevToolsWebContents()->GetPrimaryMainFrame()->ExecuteJavaScript(
-          javascript, base::NullCallback());
+  // If the devtools can dock, "SetIsDocked" will be called by devtools itself.
+  if (!can_dock_) {
+    SetIsDocked(DispatchCallback(), false);
+    if (!devtools_title_.empty()) {
+      view_->SetTitle(devtools_title_);
     }
-  }
-
-  // If CloseDevTools was called re-entrantly during the show phase (e.g. from
-  // a JS devtools-focused handler), execute the deferred close now that the
-  // focus notification stack has fully unwound.
-  if (close_devtools_pending_) {
-    close_devtools_pending_ = false;
-    CloseDevTools();
-    return;
+  } else {
+    if (dock_state_.empty()) {
+      const base::DictValue& prefs =
+          pref_service_->GetDict(kDevToolsPreferences);
+      const std::string* current_dock_state =
+          prefs.FindString("currentDockState");
+      if (current_dock_state) {
+        std::string sanitized;
+        base::RemoveChars(*current_dock_state, "\"", &sanitized);
+        dock_state_ = IsValidDockState(sanitized) ? sanitized : "right";
+      } else {
+        dock_state_ = "right";
+      }
+    }
+#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_LINUX)
+    auto* api_web_contents = api::WebContents::From(GetWebContents());
+    if (api_web_contents) {
+      auto* win =
+          static_cast<NativeWindowViews*>(api_web_contents->owner_window());
+      // When WCO is enabled, undock the devtools if the current dock
+      // position overlaps with the position of window controls to avoid
+      // broken layout.
+      if (win && win->IsWindowControlsOverlayEnabled()) {
+        if (IsAppRTL() && dock_state_ == "left") {
+          dock_state_ = "undocked";
+        } else if (dock_state_ == "right") {
+          dock_state_ = "undocked";
+        }
+      }
+    }
+#endif
+    std::u16string javascript = base::UTF8ToUTF16(
+        "EUI.DockController.DockController.instance().setDockSide(\"" +
+        dock_state_ + "\");");
+    GetDevToolsWebContents()->GetPrimaryMainFrame()->ExecuteJavaScript(
+        javascript, base::NullCallback());
   }
 
 #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)

shell/browser/ui/inspectable_web_contents.h

@@ -268,14 +268,6 @@ class InspectableWebContents
   std::unique_ptr<InspectableWebContentsView> view_;
 
   bool frontend_loaded_ = false;
-
-  // Re-entrancy guard: ShowDevTools triggers focus on the DevTools WebContents,
-  // which fires JS events whose microtask checkpoint can re-entrantly call
-  // CloseDevTools(). Destroying the WebContents or its widget while the focus
-  // notification is still iterating observers is a CHECK/UAF. These flags defer
-  // the close until the show path has fully unwound.
-  bool is_showing_devtools_ = false;
-  bool close_devtools_pending_ = false;
   scoped_refptr<content::DevToolsAgentHost> agent_host_;
   std::unique_ptr<content::DevToolsFrontendHost> frontend_host_;
   std::unique_ptr<DevToolsEmbedderMessageDispatcher>

spec/api-web-contents-spec.ts

@@ -1249,22 +1249,6 @@ describe('webContents module', () => {
       await devtoolsOpened2;
       expect(w.webContents.isDevToolsOpened()).to.be.true();
     });
-
-    it('does not crash when closing DevTools immediately after opening', async () => {
-      const w = new BrowserWindow({ show: true });
-      await w.loadURL('about:blank');
-
-      const devToolsFocused = once(w.webContents, 'devtools-focused');
-      w.webContents.openDevTools({ mode: 'detach' });
-      w.webContents.inspectElement(100, 100);
-      await devToolsFocused;
-
-      const devtoolsClosed = once(w.webContents, 'devtools-closed');
-      w.webContents.closeDevTools();
-      await devtoolsClosed;
-
-      expect(w.webContents.isDevToolsOpened()).to.be.false();
-    });
   });
 
   describe('setDevToolsTitle() API', () => {

spec/fixtures/crash-cases/webview-destroy-in-event/index.js

@@ -0,0 +1,41 @@
+const { app, BrowserWindow, webContents } = require('electron');
+
+app.whenReady().then(() => {
+  const w = new BrowserWindow({
+    show: false,
+    webPreferences: {
+      webviewTag: true,
+      nodeIntegration: true,
+      contextIsolation: false
+    }
+  });
+
+  w.loadURL('data:text/html,' + encodeURIComponent(
+    '<webview id="wv" src="about:blank" style="width:100px;height:100px"></webview>' +
+    '<script>document.getElementById("wv").addEventListener("dom-ready", function() {' +
+    '  document.title = "READY:" + this.getWebContentsId();' +
+    '});</script>'
+  ));
+
+  const check = setInterval(() => {
+    if (!w.getTitle().startsWith('READY:')) return;
+    clearInterval(check);
+    const guestId = parseInt(w.getTitle().split(':')[1]);
+    const guest = webContents.fromId(guestId);
+    if (!guest) {
+      app.quit();
+      return;
+    }
+
+    // Destroying a guest webContents inside an event handler previously caused
+    // a use-after-free because Destroy() deleted the C++ object synchronously
+    // while the event emission was still on the call stack.
+    guest.on('did-navigate-in-page', () => {
+      guest.destroy();
+    });
+
+    guest.executeJavaScript('history.pushState({}, "", "#trigger")');
+
+    setTimeout(() => app.quit(), 2000);
+  }, 200);
+});