Commit 549b3adb authored by Rouslan Solomakhin's avatar Rouslan Solomakhin Committed by Commit Bot

[Web Payment] Additional Mojo validation.

Before this patch, a Mojo message PaymentRequest.Init() with null
details, id, or total would not be rejected and could cause null pointer
dereferences in the browser.

This patch adds checks for null details, id, and total in the browser,
so the invalid Mojo IPC messages are rejected.

After this patch, a Mojo message PaymentRequest.Init() with null
details, id, or total is rejected, so the transaction is aborted.

Bug: 1148074
Change-Id: I39c821eef034cafc4ed830769785fb812304f678
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2556470Reviewed-by: default avatarLiquan (Max) Gu <maxlg@chromium.org>
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#831005}
parent ce9caebb
...@@ -329,8 +329,17 @@ public class PaymentRequestService ...@@ -329,8 +329,17 @@ public class PaymentRequestService
return null; return null;
} }
for (PaymentMethodData datum : methodData) {
if (datum == null || TextUtils.isEmpty(datum.supportedMethod)) {
abortBeforeInstantiation(client, journeyLogger,
ErrorStrings.INVALID_PAYMENT_METHODS_OR_DATA,
AbortReason.INVALID_DATA_FROM_RENDERER);
return null;
}
}
// details has default value, so could never be null, according to payment_request.idl. // details has default value, so could never be null, according to payment_request.idl.
if (details == null) { if (details == null || details.id == null || details.total == null) {
abortBeforeInstantiation(client, journeyLogger, ErrorStrings.INVALID_PAYMENT_DETAILS, abortBeforeInstantiation(client, journeyLogger, ErrorStrings.INVALID_PAYMENT_DETAILS,
AbortReason.INVALID_DATA_FROM_RENDERER); AbortReason.INVALID_DATA_FROM_RENDERER);
return null; return null;
...@@ -508,7 +517,8 @@ public class PaymentRequestService ...@@ -508,7 +517,8 @@ public class PaymentRequestService
mQueryForQuota = new HashMap<>(methodData); mQueryForQuota = new HashMap<>(methodData);
mBrowserPaymentRequest.modifyQueryForQuotaCreatedIfNeeded(mQueryForQuota, mPaymentOptions); mBrowserPaymentRequest.modifyQueryForQuotaCreatedIfNeeded(mQueryForQuota, mPaymentOptions);
if (!PaymentValidator.validatePaymentDetails(details)) { if (details == null || details.id == null || details.total == null
|| !PaymentValidator.validatePaymentDetails(details)) {
mJourneyLogger.setAborted(AbortReason.INVALID_DATA_FROM_RENDERER); mJourneyLogger.setAborted(AbortReason.INVALID_DATA_FROM_RENDERER);
disconnectFromClientWithDebugMessage( disconnectFromClientWithDebugMessage(
ErrorStrings.INVALID_PAYMENT_DETAILS, PaymentErrorReason.USER_CANCEL); ErrorStrings.INVALID_PAYMENT_DETAILS, PaymentErrorReason.USER_CANCEL);
...@@ -1118,7 +1128,9 @@ public class PaymentRequestService ...@@ -1118,7 +1128,9 @@ public class PaymentRequestService
return; return;
} }
if (!PaymentValidator.validatePaymentDetails(details) // ID cannot be updated. Updating the total is optional.
if (details == null || details.id != null
|| !PaymentValidator.validatePaymentDetails(details)
|| !mBrowserPaymentRequest.parseAndValidateDetailsFurtherIfNeeded(details)) { || !mBrowserPaymentRequest.parseAndValidateDetailsFurtherIfNeeded(details)) {
mJourneyLogger.setAborted(AbortReason.INVALID_DATA_FROM_RENDERER); mJourneyLogger.setAborted(AbortReason.INVALID_DATA_FROM_RENDERER);
disconnectFromClientWithDebugMessage( disconnectFromClientWithDebugMessage(
......
...@@ -18,10 +18,6 @@ public abstract class ErrorStrings {{ ...@@ -18,10 +18,6 @@ public abstract class ErrorStrings {{
public static final String INVALID_PAYMENT_METHODS_OR_DATA = "Invalid payment methods or data."; public static final String INVALID_PAYMENT_METHODS_OR_DATA = "Invalid payment methods or data.";
public static final String INVALID_PAYMENT_DETAILS = "Invalid payment details.";
public static final String INVALID_PAYMENT_OPTIONS = "Invalid payment options.";
public static final String INVALID_VALIDATION_ERRORS = "Invalid payment validation errors."; public static final String INVALID_VALIDATION_ERRORS = "Invalid payment validation errors.";
public static final String TAB_OVERVIEW_MODE = public static final String TAB_OVERVIEW_MODE =
......
...@@ -174,15 +174,21 @@ void PaymentRequest::Init( ...@@ -174,15 +174,21 @@ void PaymentRequest::Init(
return; return;
} }
std::string error; if (!details || !details->id || !details->total) {
if (!ValidatePaymentDetails(ConvertPaymentDetails(details), &error)) { log_.Error(errors::kInvalidPaymentDetails);
log_.Error(error); TerminateConnection();
return;
}
if (!options) {
log_.Error(errors::kInvalidPaymentOptions);
TerminateConnection(); TerminateConnection();
return; return;
} }
if (!details->total) { std::string error;
log_.Error(errors::kTotalRequired); if (!ValidatePaymentDetails(ConvertPaymentDetails(details), &error)) {
log_.Error(error);
TerminateConnection(); TerminateConnection();
return; return;
} }
...@@ -371,6 +377,13 @@ void PaymentRequest::UpdateWith(mojom::PaymentDetailsPtr details) { ...@@ -371,6 +377,13 @@ void PaymentRequest::UpdateWith(mojom::PaymentDetailsPtr details) {
return; return;
} }
// ID cannot be updated. Updating the total is optional.
if (!details || details->id) {
log_.Error(errors::kInvalidPaymentDetails);
TerminateConnection();
return;
}
std::string error; std::string error;
if (!ValidatePaymentDetails(ConvertPaymentDetails(details), &error)) { if (!ValidatePaymentDetails(ConvertPaymentDetails(details), &error)) {
log_.Error(error); log_.Error(error);
......
...@@ -37,6 +37,8 @@ const char kSkipAppForPartialDelegation[] = "Skipping $ for not providing all of ...@@ -37,6 +37,8 @@ const char kSkipAppForPartialDelegation[] = "Skipping $ for not providing all of
const char kStrictBasicCardShowReject[] = "User does not have valid information on file."; const char kStrictBasicCardShowReject[] = "User does not have valid information on file.";
const char kTotalRequired[] = "Total required."; const char kTotalRequired[] = "Total required.";
const char kUserCancelled[] = "User closed the Payment Request UI."; const char kUserCancelled[] = "User closed the Payment Request UI.";
const char kInvalidPaymentDetails[] = "Invalid payment details.";
const char kInvalidPaymentOptions[] = "Invalid payment options.";
// clang-format on // clang-format on
} // namespace errors } // namespace errors
} // namespace payments } // namespace payments
...@@ -101,6 +101,13 @@ extern const char kTotalRequired[]; ...@@ -101,6 +101,13 @@ extern const char kTotalRequired[];
// Used when user dismissed the Payment Request dialog. // Used when user dismissed the Payment Request dialog.
extern const char kUserCancelled[]; extern const char kUserCancelled[];
// Used when the renderer does not provide valid payment details, such as a null
// struct or missing ID or total.
extern const char kInvalidPaymentDetails[];
// Used when the renderer does not provide valid options, such as a null struct.
extern const char kInvalidPaymentOptions[];
} // namespace errors } // namespace errors
} // namespace payments } // namespace payments
......
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