fix: prevent use-after-free in permission request callbacks (#50032)

EnterFullscreenModeForTab, RequestPointerLock, and RequestKeyboardLock
bind callbacks with base::Unretained(this); fullscreen also captures a
raw RenderFrameHost*. These callbacks may be invoked by the app's JS
permission handler after the WebContents or RenderFrameHost is destroyed.

Use GetWeakPtr() in all three call sites, and capture a
GlobalRenderFrameHostToken instead of the raw RenderFrameHost* for
fullscreen so the pointer is resolved and null-checked only when the
callback fires. Cancel in-flight permission requests from ~WebContents()
via a new ElectronPermissionManager::CancelPendingRequests()` so stale
callbacks are never handed back to JS.
This commit is contained in:
Shelley Vohr
2026-03-03 01:01:24 +01:00
committed by GitHub
parent 21a2bfca16
commit 9f9a5b8b9b
4 changed files with 42 additions and 9 deletions

View File

@@ -98,6 +98,7 @@
#include "shell/browser/electron_browser_context.h" #include "shell/browser/electron_browser_context.h"
#include "shell/browser/electron_browser_main_parts.h" #include "shell/browser/electron_browser_main_parts.h"
#include "shell/browser/electron_navigation_throttle.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/file_select_helper.h"
#include "shell/browser/native_window.h" #include "shell/browser/native_window.h"
#include "shell/browser/osr/osr_render_widget_host_view.h" #include "shell/browser/osr/osr_render_widget_host_view.h"
@@ -1110,6 +1111,13 @@ void WebContents::InitWithWebContents(
} }
WebContents::~WebContents() { 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_) if (inspectable_web_contents_)
inspectable_web_contents_->GetView()->SetDelegate(nullptr); inspectable_web_contents_->GetView()->SetDelegate(nullptr);
@@ -1551,18 +1559,24 @@ void WebContents::EnterFullscreenModeForTab(
auto* permission_helper = auto* permission_helper =
WebContentsPermissionHelper::FromWebContents(source); WebContentsPermissionHelper::FromWebContents(source);
auto callback = auto callback =
base::BindRepeating(&WebContents::OnEnterFullscreenModeForTab, base::BindOnce(&WebContents::OnEnterFullscreenModeForTab, GetWeakPtr(),
base::Unretained(this), requesting_frame, options); requesting_frame->GetGlobalFrameToken(), options);
permission_helper->RequestFullscreenPermission(requesting_frame, callback); permission_helper->RequestFullscreenPermission(requesting_frame,
std::move(callback));
} }
void WebContents::OnEnterFullscreenModeForTab( void WebContents::OnEnterFullscreenModeForTab(
content::RenderFrameHost* requesting_frame, const content::GlobalRenderFrameHostToken& frame_token,
const blink::mojom::FullscreenOptions& options, const blink::mojom::FullscreenOptions& options,
bool allowed) { bool allowed) {
if (!allowed || !owner_window()) if (!allowed || !owner_window())
return; return;
auto* requesting_frame =
content::RenderFrameHost::FromFrameToken(frame_token);
if (!requesting_frame)
return;
auto* source = content::WebContents::FromRenderFrameHost(requesting_frame); auto* source = content::WebContents::FromRenderFrameHost(requesting_frame);
if (IsFullscreenForTabOrPending(source)) { if (IsFullscreenForTabOrPending(source)) {
DCHECK_EQ(fullscreen_frame_, source->GetFocusedFrame()); DCHECK_EQ(fullscreen_frame_, source->GetFocusedFrame());
@@ -1681,8 +1695,7 @@ void WebContents::RequestPointerLock(content::WebContents* web_contents,
WebContentsPermissionHelper::FromWebContents(web_contents); WebContentsPermissionHelper::FromWebContents(web_contents);
permission_helper->RequestPointerLockPermission( permission_helper->RequestPointerLockPermission(
user_gesture, last_unlocked_by_target, user_gesture, last_unlocked_by_target,
base::BindOnce(&WebContents::OnRequestPointerLock, base::BindOnce(&WebContents::OnRequestPointerLock, GetWeakPtr()));
base::Unretained(this)));
} }
void WebContents::LostPointerLock() { void WebContents::LostPointerLock() {
@@ -1712,8 +1725,8 @@ void WebContents::RequestKeyboardLock(content::WebContents* web_contents,
auto* permission_helper = auto* permission_helper =
WebContentsPermissionHelper::FromWebContents(web_contents); WebContentsPermissionHelper::FromWebContents(web_contents);
permission_helper->RequestKeyboardLockPermission( permission_helper->RequestKeyboardLockPermission(
esc_key_locked, base::BindOnce(&WebContents::OnRequestKeyboardLock, esc_key_locked,
base::Unretained(this))); base::BindOnce(&WebContents::OnRequestKeyboardLock, GetWeakPtr()));
} }
void WebContents::CancelKeyboardLockRequest( void WebContents::CancelKeyboardLockRequest(

View File

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

View File

@@ -169,6 +169,23 @@ bool ElectronPermissionManager::HasPermissionCheckHandler() const {
return !check_handler_.is_null(); 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( void ElectronPermissionManager::RequestPermissionWithDetails(
blink::mojom::PermissionDescriptorPtr permission, blink::mojom::PermissionDescriptorPtr permission,
content::RenderFrameHost* render_frame_host, content::RenderFrameHost* render_frame_host,

View File

@@ -86,6 +86,8 @@ class ElectronPermissionManager : public content::PermissionControllerDelegate {
bool HasPermissionRequestHandler() const; bool HasPermissionRequestHandler() const;
bool HasPermissionCheckHandler() const; bool HasPermissionCheckHandler() const;
void CancelPendingRequests(content::WebContents* web_contents);
void CheckBluetoothDevicePair(gin_helper::Dictionary details, void CheckBluetoothDevicePair(gin_helper::Dictionary details,
PairCallback pair_callback) const; PairCallback pair_callback) const;