Commit 9244d6d8 authored by Rouslan Solomakhin's avatar Rouslan Solomakhin Committed by Commit Bot

[Web Payment] Cancel pending database operations.

Before this patch, payment credential storage and retrieval did not
cancel database operations when secure payment confirmation was being
torn down, so the database operation callbacks could use secure payment
confirmation code path after free.

This patch cancels the payment credential storage and retrieval
operations during the tear down.

After this patch, there is no use-after-free errors when a database
operation is in flight during secure payment confirmation tear down.

Bug: 1136078
Change-Id: I2a02089f4ab5c3c8ae5ebed91304b11e1c1eaf36
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2518924Reviewed-by: default avatarNick Burris <nburris@chromium.org>
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824186}
parent 0fd68c17
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "components/payments/content/payment_credential.h" #include "components/payments/content/payment_credential.h"
#include <algorithm>
#include <memory> #include <memory>
#include "base/memory/ref_counted_memory.h" #include "base/memory/ref_counted_memory.h"
...@@ -19,16 +20,22 @@ namespace payments { ...@@ -19,16 +20,22 @@ namespace payments {
PaymentCredential::PaymentCredential( PaymentCredential::PaymentCredential(
content::WebContents* web_contents, content::WebContents* web_contents,
content::GlobalFrameRoutingId initiator_frame_routing_id, content::GlobalFrameRoutingId initiator_frame_routing_id,
scoped_refptr<PaymentManifestWebDataService> web_data_sevice, scoped_refptr<PaymentManifestWebDataService> web_data_service,
mojo::PendingReceiver<mojom::PaymentCredential> receiver) mojo::PendingReceiver<mojom::PaymentCredential> receiver)
: WebContentsObserver(web_contents), : WebContentsObserver(web_contents),
initiator_frame_routing_id_(initiator_frame_routing_id), initiator_frame_routing_id_(initiator_frame_routing_id),
web_data_service_(web_data_sevice) { web_data_service_(web_data_service) {
DCHECK(web_contents); DCHECK(web_contents);
receiver_.Bind(std::move(receiver)); receiver_.Bind(std::move(receiver));
} }
PaymentCredential::~PaymentCredential() = default; PaymentCredential::~PaymentCredential() {
if (web_data_service_) {
std::for_each(callbacks_.begin(), callbacks_.end(), [&](const auto& pair) {
web_data_service_->CancelRequest(pair.first);
});
}
}
void PaymentCredential::StorePaymentCredential( void PaymentCredential::StorePaymentCredential(
payments::mojom::PaymentCredentialInstrumentPtr instrument, payments::mojom::PaymentCredentialInstrumentPtr instrument,
......
...@@ -39,7 +39,7 @@ class PaymentCredential : public mojom::PaymentCredential, ...@@ -39,7 +39,7 @@ class PaymentCredential : public mojom::PaymentCredential,
PaymentCredential( PaymentCredential(
content::WebContents* web_contents, content::WebContents* web_contents,
content::GlobalFrameRoutingId initiator_frame_routing_id, content::GlobalFrameRoutingId initiator_frame_routing_id,
scoped_refptr<PaymentManifestWebDataService> web_data_sevice, scoped_refptr<PaymentManifestWebDataService> web_data_service,
mojo::PendingReceiver<mojom::PaymentCredential> receiver); mojo::PendingReceiver<mojom::PaymentCredential> receiver);
~PaymentCredential() override; ~PaymentCredential() override;
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "components/payments/content/secure_payment_confirmation_app_factory.h" #include "components/payments/content/secure_payment_confirmation_app_factory.h"
#include <stdint.h> #include <stdint.h>
#include <algorithm>
#include <memory> #include <memory>
#include <string> #include <string>
#include <utility> #include <utility>
...@@ -97,15 +98,19 @@ void SecurePaymentConfirmationAppFactory:: ...@@ -97,15 +98,19 @@ void SecurePaymentConfirmationAppFactory::
WebDataServiceBase::Handle handle = WebDataServiceBase::Handle handle =
web_data_service->GetSecurePaymentConfirmationInstruments( web_data_service->GetSecurePaymentConfirmationInstruments(
std::move(request->credential_ids), this); std::move(request->credential_ids), this);
requests_[handle] = std::make_unique<Request>(delegate, std::move(request), requests_[handle] = std::make_unique<Request>(
std::move(authenticator)); delegate, web_data_service, std::move(request), std::move(authenticator));
} }
SecurePaymentConfirmationAppFactory::SecurePaymentConfirmationAppFactory() SecurePaymentConfirmationAppFactory::SecurePaymentConfirmationAppFactory()
: PaymentAppFactory(PaymentApp::Type::INTERNAL) {} : PaymentAppFactory(PaymentApp::Type::INTERNAL) {}
SecurePaymentConfirmationAppFactory::~SecurePaymentConfirmationAppFactory() = SecurePaymentConfirmationAppFactory::~SecurePaymentConfirmationAppFactory() {
default; std::for_each(requests_.begin(), requests_.end(), [&](const auto& pair) {
if (pair.second->web_data_service)
pair.second->web_data_service->CancelRequest(pair.first);
});
}
void SecurePaymentConfirmationAppFactory::Create( void SecurePaymentConfirmationAppFactory::Create(
base::WeakPtr<Delegate> delegate) { base::WeakPtr<Delegate> delegate) {
...@@ -147,11 +152,14 @@ void SecurePaymentConfirmationAppFactory::Create( ...@@ -147,11 +152,14 @@ void SecurePaymentConfirmationAppFactory::Create(
struct SecurePaymentConfirmationAppFactory::Request struct SecurePaymentConfirmationAppFactory::Request
: public content::WebContentsObserver { : public content::WebContentsObserver {
Request(base::WeakPtr<PaymentAppFactory::Delegate> delegate, Request(
mojom::SecurePaymentConfirmationRequestPtr mojo_request, base::WeakPtr<PaymentAppFactory::Delegate> delegate,
std::unique_ptr<autofill::InternalAuthenticator> authenticator) scoped_refptr<payments::PaymentManifestWebDataService> web_data_service,
mojom::SecurePaymentConfirmationRequestPtr mojo_request,
std::unique_ptr<autofill::InternalAuthenticator> authenticator)
: content::WebContentsObserver(delegate->GetWebContents()), : content::WebContentsObserver(delegate->GetWebContents()),
delegate(delegate), delegate(delegate),
web_data_service(web_data_service),
mojo_request(std::move(mojo_request)), mojo_request(std::move(mojo_request)),
authenticator(std::move(authenticator)) {} authenticator(std::move(authenticator)) {}
...@@ -161,6 +169,7 @@ struct SecurePaymentConfirmationAppFactory::Request ...@@ -161,6 +169,7 @@ struct SecurePaymentConfirmationAppFactory::Request
Request& operator=(const Request& other) = delete; Request& operator=(const Request& other) = delete;
base::WeakPtr<PaymentAppFactory::Delegate> delegate; base::WeakPtr<PaymentAppFactory::Delegate> delegate;
scoped_refptr<payments::PaymentManifestWebDataService> web_data_service;
mojom::SecurePaymentConfirmationRequestPtr mojo_request; mojom::SecurePaymentConfirmationRequestPtr mojo_request;
std::unique_ptr<autofill::InternalAuthenticator> authenticator; std::unique_ptr<autofill::InternalAuthenticator> authenticator;
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment