Skip to content

Commit 7046b3c

Browse files
nornagonnornagon
authored
chore: cherry-pick 686d1bfbcb8f from chromium (electron#23457)
1 parent f450985 commit 7046b3c

File tree

2 files changed

+145
-0
lines changed

2 files changed

+145
-0
lines changed

‎patches/chromium/.patches

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,3 +115,4 @@ when_suspending_context_don_t_clear_handlers.patch
115115
use_keepselfalive_on_audiocontext_to_keep_it_alive_until_rendering.patch
116116
worker_stop_passing_creator_s_origin_for_starting_a_dedicated_worker.patch
117117
cherry-pick-b69991a9b701.patch
118+
cherry-pick-686d1bfbcb8f.patch

‎patches/chromium/cherry-pick-686d1bfbcb8f.patch

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
2+
From: Rouslan Solomakhin <rouslan@chromium.org>
3+
Date: Wed, 15 Apr 2020 23:03:07 +0000
4+
Subject: Browser context owned callback.
5+
6+
Before this patch, an unowned function pointer would be invoked
7+
asynchronously with a reference to the possibly freed reference to the
8+
browser context, which could cause use after free in certain
9+
circumstances.
10+
11+
This patch makes the browser context own the callback and binds the
12+
function with a weak pointer, so freeing the browser context invalidates
13+
the weak pointer, which cancels the callback execution.
14+
15+
After this patch, freeing the browser context aborts the asynchronous
16+
callback that dereferences the browser context, so the use after free
17+
is prevented.
18+
19+
TBR=rouslan@chromium.org
20+
21+
(cherry picked from commit 2d0aad1e7602a7076d86772cc159b891cf2cf03b)
22+
23+
Bug: 1065298
24+
Change-Id: Id6de3099a55c4505e94a8a6d21fb25d6d2b34c6c
25+
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2144311
26+
Reviewed-by: Danyao Wang <danyao@chromium.org>
27+
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
28+
Cr-Original-Commit-Position: refs/heads/master@{#758404}
29+
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2151474
30+
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
31+
Cr-Commit-Position: refs/branch-heads/4044@{#942}
32+
Cr-Branched-From: a6d9daf149a473ceea37f629c41d4527bf2055bd-refs/heads/master@{#737173}
33+
34+
diff --git a/content/browser/payments/payment_app_provider_impl.cc b/content/browser/payments/payment_app_provider_impl.cc
35+
index 9a84066912f0cef926a941a69090af3749d09288..694d7f57ac3814cb8d99ceecd4d90477a4bdd26f 100644
36+
--- a/content/browser/payments/payment_app_provider_impl.cc
37+
+++ b/content/browser/payments/payment_app_provider_impl.cc
38+
@@ -15,6 +15,7 @@
39+
#include "base/metrics/histogram_macros.h"
40+
#include "base/strings/string_number_conversions.h"
41+
#include "base/strings/string_util.h"
42+
+#include "base/supports_user_data.h"
43+
#include "base/task/post_task.h"
44+
#include "base/token.h"
45+
#include "content/browser/payments/payment_app_context_impl.h"
46+
@@ -425,28 +426,65 @@ void OnInstallPaymentApp(
47+
}
48+
}
49+
50+
-void CheckPermissionForPaymentApps(
51+
- BrowserContext* browser_context,
52+
- PaymentAppProvider::GetAllPaymentAppsCallback callback,
53+
- PaymentAppProvider::PaymentApps apps) {
54+
- DCHECK_CURRENTLY_ON(BrowserThread::UI);
55+
+// Callbacks for checking permissions asynchronously. Owned by the browser
56+
+// context to avoid using the browser context after it has been freed. Deleted
57+
+// after the callback is invoked.
58+
+// Sample usage:
59+
+// PostTask(&PermissionChecker::CheckPermissionForPaymentApps,
60+
+// PermissionChecker::Create(browser_context), std::move(callback));
61+
+class PermissionChecker : public base::SupportsUserData::Data {
62+
+ public:
63+
+ static base::WeakPtr<PermissionChecker> Create(
64+
+ BrowserContext* browser_context) {
65+
+ auto owned = std::make_unique<PermissionChecker>(browser_context);
66+
+ auto weak_pointer_result = owned->weak_ptr_factory_.GetWeakPtr();
67+
+ void* key = owned.get();
68+
+ browser_context->SetUserData(key, std::move(owned));
69+
+ return weak_pointer_result;
70+
+ }
71+
+
72+
+ // Do not use this method directly! Use the static PermissionChecker::Create()
73+
+ // method instead. (The constructor must be public for std::make_unique<> in
74+
+ // the Create() method.)
75+
+ explicit PermissionChecker(BrowserContext* browser_context)
76+
+ : browser_context_(browser_context) {}
77+
+ ~PermissionChecker() override = default;
78+
+
79+
+ // Disallow copy and assign.
80+
+ PermissionChecker(const PermissionChecker& other) = delete;
81+
+ PermissionChecker& operator=(const PermissionChecker& other) = delete;
82+
+
83+
+ void CheckPermissionForPaymentApps(
84+
+ PaymentAppProvider::GetAllPaymentAppsCallback callback,
85+
+ PaymentAppProvider::PaymentApps apps) {
86+
+ DCHECK_CURRENTLY_ON(BrowserThread::UI);
87+
88+
- PermissionController* permission_controller =
89+
- BrowserContext::GetPermissionController(browser_context);
90+
- DCHECK(permission_controller);
91+
-
92+
- PaymentAppProvider::PaymentApps permitted_apps;
93+
- for (auto& app : apps) {
94+
- GURL origin = app.second->scope.GetOrigin();
95+
- if (permission_controller->GetPermissionStatus(
96+
- PermissionType::PAYMENT_HANDLER, origin, origin) ==
97+
- blink::mojom::PermissionStatus::GRANTED) {
98+
- permitted_apps[app.first] = std::move(app.second);
99+
+ PermissionController* permission_controller =
100+
+ BrowserContext::GetPermissionController(browser_context_);
101+
+ DCHECK(permission_controller);
102+
+
103+
+ PaymentAppProvider::PaymentApps permitted_apps;
104+
+ for (auto& app : apps) {
105+
+ GURL origin = app.second->scope.GetOrigin();
106+
+ if (permission_controller->GetPermissionStatus(
107+
+ PermissionType::PAYMENT_HANDLER, origin, origin) ==
108+
+ blink::mojom::PermissionStatus::GRANTED) {
109+
+ permitted_apps[app.first] = std::move(app.second);
110+
+ }
111+
}
112+
+
113+
+ std::move(callback).Run(std::move(permitted_apps));
114+
+
115+
+ // Deletes this PermissionChecker object.
116+
+ browser_context_->RemoveUserData(/*key=*/this);
117+
}
118+
119+
- std::move(callback).Run(std::move(permitted_apps));
120+
-}
121+
+ private:
122+
+ // Owns this PermissionChecker object, so it's always valid.
123+
+ BrowserContext* browser_context_;
124+
+
125+
+ base::WeakPtrFactory<PermissionChecker> weak_ptr_factory_{this};
126+
+};
127+
128+
void AbortInvokePaymentApp(BrowserContext* browser_context,
129+
PaymentEventResponseType reason) {
130+
@@ -599,9 +637,11 @@ void PaymentAppProviderImpl::GetAllPaymentApps(
131+
132+
RunOrPostTaskOnThread(
133+
FROM_HERE, ServiceWorkerContext::GetCoreThreadId(),
134+
- base::BindOnce(&GetAllPaymentAppsOnCoreThread, payment_app_context,
135+
- base::BindOnce(&CheckPermissionForPaymentApps,
136+
- browser_context, std::move(callback))));
137+
+ base::BindOnce(
138+
+ &GetAllPaymentAppsOnCoreThread, payment_app_context,
139+
+ base::BindOnce(&PermissionChecker::CheckPermissionForPaymentApps,
140+
+ PermissionChecker::Create(browser_context),
141+
+ std::move(callback))));
142+
}
143+
144+
void PaymentAppProviderImpl::InvokePaymentApp(

0 commit comments

Comments
 (0)