Repository: electron/electron
Pull request: fix: prevent use-after-free in permission request callbacks

Backport of #50032

See that PR for details.


Notes: none


Changed files:
filename: shell/browser/api/electron_api_web_contents.cc
patch: @@ -96,6 +96,7 @@
 #include "shell/browser/electron_browser_context.h"
 #include "shell/browser/electron_browser_main_parts.h"
 #include "shell/browser/electron_navigation_throttle.h"
+#include "shell/browser/electron_permission_manager.h"
 #include "shell/browser/file_select_helper.h"
 #include "shell/browser/native_window.h"
 #include "shell/browser/osr/osr_render_widget_host_view.h"
@@ -1048,6 +1049,13 @@ void WebContents::InitWithWebContents(
 }
 
 WebContents::~WebContents() {
+  if (web_contents()) {
+    auto* permission_manager = static_cast<ElectronPermissionManager*>(
+        web_contents()->GetBrowserContext()->GetPermissionControllerDelegate());
+    if (permission_manager)
+      permission_manager->CancelPendingRequests(web_contents());
+  }
+
   if (inspectable_web_contents_)
     inspectable_web_contents_->GetView()->SetDelegate(nullptr);
 
@@ -1477,18 +1485,24 @@ void WebContents::EnterFullscreenModeForTab(
   auto* permission_helper =
       WebContentsPermissionHelper::FromWebContents(source);
   auto callback =
-      base::BindRepeating(&WebContents::OnEnterFullscreenModeForTab,
-                          base::Unretained(this), requesting_frame, options);
-  permission_helper->RequestFullscreenPermission(requesting_frame, callback);
+      base::BindOnce(&WebContents::OnEnterFullscreenModeForTab, GetWeakPtr(),
+                     requesting_frame->GetGlobalFrameToken(), options);
+  permission_helper->RequestFullscreenPermission(requesting_frame,
+                                                 std::move(callback));
 }
 
 void WebContents::OnEnterFullscreenModeForTab(
-    content::RenderFrameHost* requesting_frame,
+    const content::GlobalRenderFrameHostToken& frame_token,
     const blink::mojom::FullscreenOptions& options,
     bool allowed) {
   if (!allowed || !owner_window())
     return;
 
+  auto* requesting_frame =
+      content::RenderFrameHost::FromFrameToken(frame_token);
+  if (!requesting_frame)
+    return;
+
   auto* source = content::WebContents::FromRenderFrameHost(requesting_frame);
   if (IsFullscreenForTabOrPending(source)) {
     DCHECK_EQ(fullscreen_frame_, source->GetFocusedFrame());
@@ -1607,8 +1621,7 @@ void WebContents::RequestPointerLock(content::WebContents* web_contents,
       WebContentsPermissionHelper::FromWebContents(web_contents);
   permission_helper->RequestPointerLockPermission(
       user_gesture, last_unlocked_by_target,
-      base::BindOnce(&WebContents::OnRequestPointerLock,
-                     base::Unretained(this)));
+      base::BindOnce(&WebContents::OnRequestPointerLock, GetWeakPtr()));
 }
 
 void WebContents::LostPointerLock() {
@@ -1638,8 +1651,8 @@ void WebContents::RequestKeyboardLock(content::WebContents* web_contents,
   auto* permission_helper =
       WebContentsPermissionHelper::FromWebContents(web_contents);
   permission_helper->RequestKeyboardLockPermission(
-      esc_key_locked, base::BindOnce(&WebContents::OnRequestKeyboardLock,
-                                     base::Unretained(this)));
+      esc_key_locked,
+      base::BindOnce(&WebContents::OnRequestKeyboardLock, GetWeakPtr()));
 }
 
 void WebContents::CancelKeyboardLockRequest(

filename: shell/browser/api/electron_api_web_contents.h
patch: @@ -24,6 +24,7 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/devtools_agent_host.h"
 #include "content/public/browser/frame_tree_node_id.h"
+#include "content/public/browser/global_routing_id.h"
 #include "content/public/browser/javascript_dialog_manager.h"
 #include "content/public/browser/render_widget_host.h"
 #include "content/public/browser/web_contents_delegate.h"
@@ -336,7 +337,7 @@ class WebContents final : public ExclusiveAccessContext,
 
   // Callback triggered on permission response.
   void OnEnterFullscreenModeForTab(
-      content::RenderFrameHost* requesting_frame,
+      const content::GlobalRenderFrameHostToken& frame_token,
       const blink::mojom::FullscreenOptions& options,
       bool allowed);
 

filename: shell/browser/electron_permission_manager.cc
patch: @@ -154,6 +154,23 @@ bool ElectronPermissionManager::HasPermissionCheckHandler() const {
   return !check_handler_.is_null();
 }
 
+void ElectronPermissionManager::CancelPendingRequests(
+    content::WebContents* web_contents) {
+  std::vector<int> ids_to_remove;
+  for (PendingRequestsMap::iterator iter(&pending_requests_); !iter.IsAtEnd();
+       iter.Advance()) {
+    auto* pending_request = iter.GetCurrentValue();
+    content::RenderFrameHost* rfh = pending_request->GetRenderFrameHost();
+    if (!rfh ||
+        content::WebContents::FromRenderFrameHost(rfh) == web_contents) {
+      ids_to_remove.push_back(iter.GetCurrentKey());
+    }
+  }
+  for (int id : ids_to_remove) {
+    pending_requests_.Remove(id);
+  }
+}
+
 void ElectronPermissionManager::RequestPermissionWithDetails(
     blink::mojom::PermissionDescriptorPtr permission,
     content::RenderFrameHost* render_frame_host,

filename: shell/browser/electron_permission_manager.h
patch: @@ -84,6 +84,8 @@ class ElectronPermissionManager : public content::PermissionControllerDelegate {
   bool HasPermissionRequestHandler() const;
   bool HasPermissionCheckHandler() const;
 
+  void CancelPendingRequests(content::WebContents* web_contents);
+
   void CheckBluetoothDevicePair(gin_helper::Dictionary details,
                                 PairCallback pair_callback) const;