github/electron
1
1
Fork 0
mirror of https://github.com/electron/electron.git synced 2026-04-10 03:01:51 -04:00
Code Issues Packages Projects Releases Wiki Activity

chore: cherry-pick 686d1bfbcb8f from chromium (#23458)

Browse Source
This commit is contained in:
Jeremy Apthorp
2020-05-08 11:20:17 -07:00
committed by GitHub
parent f452b9f9ac
commit 12ea5a028d
2 changed files with 145 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
patches/common/chromium/.patches
View File

@@ -110,4 +110,5 @@ speculative_fix_for_crashes_in_filechooserimpl.patch
reland_sequentialise_access_to_callbacks_in.patch
handle_err_cache_race_in_dodoneheadersaddtoentrycomplete.patch
mojovideoencodeacceleratorservice_handle_potential_later.patch
cherry-pick-686d1bfbcb8f.patch
cherry-pick-b69991a9b701.patch

144
patches/common/chromium/cherry-pick-686d1bfbcb8f.patch Normal file
View File

@@ -0,0 +1,144 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Rouslan Solomakhin <rouslan@chromium.org>
Date: Wed, 15 Apr 2020 23:03:07 +0000
Subject: Browser context owned callback.
Before this patch, an unowned function pointer would be invoked
asynchronously with a reference to the possibly freed reference to the
browser context, which could cause use after free in certain
circumstances.
This patch makes the browser context own the callback and binds the
function with a weak pointer, so freeing the browser context invalidates
the weak pointer, which cancels the callback execution.
After this patch, freeing the browser context aborts the asynchronous
callback that dereferences the browser context, so the use after free
is prevented.
TBR=rouslan@chromium.org
(cherry picked from commit 2d0aad1e7602a7076d86772cc159b891cf2cf03b)
Bug: 1065298
Change-Id: Id6de3099a55c4505e94a8a6d21fb25d6d2b34c6c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2144311
Reviewed-by: Danyao Wang <danyao@chromium.org>
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#758404}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2151474
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/branch-heads/4044@{#942}
Cr-Branched-From: a6d9daf149a473ceea37f629c41d4527bf2055bd-refs/heads/master@{#737173}
diff --git a/content/browser/payments/payment_app_provider_impl.cc b/content/browser/payments/payment_app_provider_impl.cc
index 88643bbaa7f37b3005d8a247f87543b9cf5f01e6..0c3f4b4966a6b2017a143f306f4f6269b35f7f02 100644
--- a/content/browser/payments/payment_app_provider_impl.cc
+++ b/content/browser/payments/payment_app_provider_impl.cc
@@ -8,6 +8,7 @@
#include "base/bind.h"
#include "base/bind_helpers.h"
#include "base/strings/string_util.h"
+#include "base/supports_user_data.h"
#include "base/task/post_task.h"
#include "content/browser/payments/payment_app_context_impl.h"
#include "content/browser/payments/payment_app_installer.h"
@@ -393,28 +394,65 @@ void OnInstallPaymentApp(payments::mojom::PaymentRequestEventDataPtr event_data,
}
}
-void CheckPermissionForPaymentApps(
- BrowserContext* browser_context,
- PaymentAppProvider::GetAllPaymentAppsCallback callback,
- PaymentAppProvider::PaymentApps apps) {
- DCHECK_CURRENTLY_ON(BrowserThread::UI);
+// Callbacks for checking permissions asynchronously. Owned by the browser
+// context to avoid using the browser context after it has been freed. Deleted
+// after the callback is invoked.
+// Sample usage:
+// PostTask(&PermissionChecker::CheckPermissionForPaymentApps,
+// PermissionChecker::Create(browser_context), std::move(callback));
+class PermissionChecker : public base::SupportsUserData::Data {
+ public:
+ static base::WeakPtr<PermissionChecker> Create(
+ BrowserContext* browser_context) {
+ auto owned = std::make_unique<PermissionChecker>(browser_context);
+ auto weak_pointer_result = owned->weak_ptr_factory_.GetWeakPtr();
+ void* key = owned.get();
+ browser_context->SetUserData(key, std::move(owned));
+ return weak_pointer_result;
+ }
+
+ // Do not use this method directly! Use the static PermissionChecker::Create()
+ // method instead. (The constructor must be public for std::make_unique<> in
+ // the Create() method.)
+ explicit PermissionChecker(BrowserContext* browser_context)
+ : browser_context_(browser_context) {}
+ ~PermissionChecker() override = default;
- PermissionController* permission_controller =
- BrowserContext::GetPermissionController(browser_context);
- DCHECK(permission_controller);
-
- PaymentAppProvider::PaymentApps permitted_apps;
- for (auto& app : apps) {
- GURL origin = app.second->scope.GetOrigin();
- if (permission_controller->GetPermissionStatus(
- PermissionType::PAYMENT_HANDLER, origin, origin) ==
- blink::mojom::PermissionStatus::GRANTED) {
- permitted_apps[app.first] = std::move(app.second);
+ // Disallow copy and assign.
+ PermissionChecker(const PermissionChecker& other) = delete;
+ PermissionChecker& operator=(const PermissionChecker& other) = delete;
+
+ void CheckPermissionForPaymentApps(
+ PaymentAppProvider::GetAllPaymentAppsCallback callback,
+ PaymentAppProvider::PaymentApps apps) {
+ DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+ PermissionController* permission_controller =
+ BrowserContext::GetPermissionController(browser_context_);
+ DCHECK(permission_controller);
+
+ PaymentAppProvider::PaymentApps permitted_apps;
+ for (auto& app : apps) {
+ GURL origin = app.second->scope.GetOrigin();
+ if (permission_controller->GetPermissionStatus(
+ PermissionType::PAYMENT_HANDLER, origin, origin) ==
+ blink::mojom::PermissionStatus::GRANTED) {
+ permitted_apps[app.first] = std::move(app.second);
+ }
}
+
+ std::move(callback).Run(std::move(permitted_apps));
+
+ // Deletes this PermissionChecker object.
+ browser_context_->RemoveUserData(/*key=*/this);
}
- std::move(callback).Run(std::move(permitted_apps));
-}
+ private:
+ // Owns this PermissionChecker object, so it's always valid.
+ BrowserContext* browser_context_;
+
+ base::WeakPtrFactory<PermissionChecker> weak_ptr_factory_{this};
+};
void AbortInvokePaymentApp(BrowserContext* browser_context) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -451,9 +489,11 @@ void PaymentAppProviderImpl::GetAllPaymentApps(
base::PostTaskWithTraits(
FROM_HERE, {BrowserThread::IO},
- base::BindOnce(&GetAllPaymentAppsOnIO, payment_app_context,
- base::BindOnce(&CheckPermissionForPaymentApps,
- browser_context, std::move(callback))));
+ base::BindOnce(
+ &GetAllPaymentAppsOnIO, payment_app_context,
+ base::BindOnce(&PermissionChecker::CheckPermissionForPaymentApps,
+ PermissionChecker::Create(browser_context),
+ std::move(callback))));
}
void PaymentAppProviderImpl::InvokePaymentApp(