mirror of
https://github.com/electron/electron.git
synced 2026-04-10 03:01:51 -04:00
fix: prevent use-after-free in permission request callbacks (#50034)
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. Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
This commit is contained in:
@@ -97,6 +97,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"
|
||||
@@ -1106,6 +1107,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);
|
||||
|
||||
@@ -1534,18 +1542,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());
|
||||
@@ -1664,8 +1678,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() {
|
||||
@@ -1695,8 +1708,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(
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -169,6 +169,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,
|
||||
|
||||
@@ -86,6 +86,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;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user