Don't crash web payments mojo on very long ID.
Before this patch, invoking PaymentRequest with a 10mln character value
for the "id" parameter would crash the browser on Android. The browser
was running out of memory while trying to allocate a Java String.
This patch adds protective limits of 1024 characters to all strings in
PaymentRequest that will be sent to the browser side. Lists also have
protective limits of 1024 items. The stringified "data" object cannot
exceed 1024*1024 characters in length upon serialization.
After this patch, invoking PaymentRequest with a >1024 character value
for the "id" parameter will throw a TypeError exception in JavaScript.
Spec pull request:
https://github.com/w3c/browser-payment-api/pull/540
Bug: 725744
Change-Id: I25551e7145d9d8b59d111a89a7aa6c343c45600a
Reviewed-on: https://chromium-review.googlesource.com/516882
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: Ganggui Tang <gogerald@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#476996}diff --git a/chrome/android/java_sources.gni b/chrome/android/java_sources.gni
index 0f147bc8..72d35cd 100644
--- a/chrome/android/java_sources.gni
+++ b/chrome/android/java_sources.gni
@@ -1519,6 +1519,7 @@
"javatests/src/org/chromium/chrome/browser/payments/PaymentRequestContactDetailsAndFreeShippingTest.java",
"javatests/src/org/chromium/chrome/browser/payments/PaymentRequestContactDetailsSectionUnitTest.java",
"javatests/src/org/chromium/chrome/browser/payments/PaymentRequestContactDetailsTest.java",
+ "javatests/src/org/chromium/chrome/browser/payments/PaymentRequestLongIdTest.java",
"javatests/src/org/chromium/chrome/browser/payments/PaymentRequestDataUrlTest.java",
"javatests/src/org/chromium/chrome/browser/payments/PaymentRequestDynamicShippingMultipleAddressesTest.java",
"javatests/src/org/chromium/chrome/browser/payments/PaymentRequestDynamicShippingSingleAddressTest.java",
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestLongIdTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestLongIdTest.java
new file mode 100644
index 0000000..5a79ac3a
--- /dev/null
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestLongIdTest.java
@@ -0,0 +1,46 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.payments;
+
+import android.support.test.filters.MediumTest;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.chromium.base.test.util.CommandLineFlags;
+import org.chromium.base.test.util.Feature;
+import org.chromium.chrome.browser.ChromeSwitches;
+import org.chromium.chrome.browser.payments.PaymentRequestTestRule.MainActivityStartCallback;
+import org.chromium.chrome.test.ChromeActivityTestRule;
+import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
+
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeoutException;
+
+/** A test to verify that PaymentRequest does not crash with very long request identifiers. */
+@RunWith(ChromeJUnit4ClassRunner.class)
+@CommandLineFlags.Add({
+ ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE,
+ ChromeActivityTestRule.DISABLE_NETWORK_PREDICTION_FLAG,
+})
+public class PaymentRequestLongIdTest implements MainActivityStartCallback {
+ @Rule
+ public PaymentRequestTestRule mPaymentRequestTestRule =
+ new PaymentRequestTestRule("payment_request_long_id_test.html", this);
+
+ @Override
+ public void onMainActivityStarted()
+ throws InterruptedException, ExecutionException, TimeoutException {}
+
+ @Test
+ @MediumTest
+ @Feature({"Payments"})
+ public void testNoCrash() throws InterruptedException, ExecutionException, TimeoutException {
+ mPaymentRequestTestRule.openPageAndClickNode("buy");
+ mPaymentRequestTestRule.expectResultContains(
+ new String[] {"ID cannot be longer than 1024 characters"});
+ }
+}
diff --git a/chrome/test/data/payments/long_id.js b/chrome/test/data/payments/long_id.js
new file mode 100644
index 0000000..426874f
--- /dev/null
+++ b/chrome/test/data/payments/long_id.js
@@ -0,0 +1,30 @@
+/*
+ * Copyright 2017 The Chromium Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+/** Invokes PaymentRequest with a very long request identifier. */
+function buy() { // eslint-disable-line no-unused-vars
+ const foo = Object.freeze({
+ supportedMethods: ['basic-card']
+ });
+ const defaultMethods = Object.freeze([foo]);
+ const defaultDetails = Object.freeze({
+ total: {
+ label: 'Label',
+ amount: {
+ currency: 'USD',
+ value: '1.00',
+ },
+ },
+ });
+ const newDetails = Object.assign({}, defaultDetails, {
+ id: ''.padStart(100000000, '\n very long id \t \n '),
+ });
+ try {
+ const request = new PaymentRequest(defaultMethods, newDetails);
+ } catch (error) {
+ print(error);
+ }
+}
diff --git a/chrome/test/data/payments/payment_request_long_id_test.html b/chrome/test/data/payments/payment_request_long_id_test.html
new file mode 100644
index 0000000..7417b0bb
--- /dev/null
+++ b/chrome/test/data/payments/payment_request_long_id_test.html
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<!--
+Copyright 2017 The Chromium Authors. All rights reserved.
+Use of this source code is governed by a BSD-style license that can be
+found in the LICENSE file.
+-->
+<html>
+
+<head>
+ <meta charset="utf-8">
+ <title>Long ID Test</title>
+ <meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1">
+ <link rel="stylesheet" type="text/css" href="style.css">
+</head>
+
+<body>
+ <button onclick="buy()" id="buy">Long ID Test</button><br>
+ <pre id="result"></pre>
+ <script src="util.js"></script>
+ <script src="long_id.js"></script>
+</body>
+
+</html>
diff --git a/third_party/WebKit/LayoutTests/external/wpt/payment-request/payment-request-constructor.https.html b/third_party/WebKit/LayoutTests/external/wpt/payment-request/payment-request-constructor.https.html
index 35fb974..0c2f687 100644
--- a/third_party/WebKit/LayoutTests/external/wpt/payment-request/payment-request-constructor.https.html
+++ b/third_party/WebKit/LayoutTests/external/wpt/payment-request/payment-request-constructor.https.html
@@ -41,7 +41,7 @@
test(() => {
const newDetails = Object.assign({}, defaultDetails, {
- id: "".padStart(100000000, "\n test 123 \t \n "),
+ id: "".padStart(1024, "\n test 123 \t \n "),
});
const request = new PaymentRequest(defaultMethods, newDetails);
assert_equals(
diff --git a/third_party/WebKit/Source/modules/payments/PaymentRequest.cpp b/third_party/WebKit/Source/modules/payments/PaymentRequest.cpp
index 2fb7103..979a04d 100644
--- a/third_party/WebKit/Source/modules/payments/PaymentRequest.cpp
+++ b/third_party/WebKit/Source/modules/payments/PaymentRequest.cpp
@@ -148,6 +148,10 @@
// resolved, then behave as if the website called complete("fail").
static const int kCompleteTimeoutSeconds = 60;
+static const size_t kMaxStringLength = 1024;
+static const size_t kMaxJSONStringLength = 1048576;
+static const size_t kMaxListSize = 1024;
+
// Validates ShippingOption or PaymentItem, which happen to have identical
// fields, except for "id", which is present only in ShippingOption.
template <typename T>
@@ -160,6 +164,30 @@
DCHECK(item.amount().hasValue());
DCHECK(item.amount().hasCurrency());
+ if (item.label().length() > kMaxStringLength) {
+ exception_state.ThrowTypeError("The label for " + item_name +
+ " cannot be longer than 1024 characters");
+ return;
+ }
+
+ if (item.amount().currency().length() > kMaxStringLength) {
+ exception_state.ThrowTypeError("The currency code for " + item_name +
+ " cannot be longer than 1024 characters");
+ return;
+ }
+
+ if (item.amount().currencySystem().length() > kMaxStringLength) {
+ exception_state.ThrowTypeError("The currency system for " + item_name +
+ " cannot be longer than 1024 characters");
+ return;
+ }
+
+ if (item.amount().value().length() > kMaxStringLength) {
+ exception_state.ThrowTypeError("The amount value for " + item_name +
+ " cannot be longer than 1024 characters");
+ return;
+ }
+
String error_message;
if (!PaymentsValidators::IsValidAmountFormat(item.amount().value(), item_name,
&error_message)) {
@@ -187,6 +215,11 @@
Vector<PaymentItemPtr>& output,
ExecutionContext& execution_context,
ExceptionState& exception_state) {
+ if (input.size() > kMaxListSize) {
+ exception_state.ThrowTypeError("At most 1024 " + item_names + " allowed");
+ return;
+ }
+
for (const PaymentItem& item : input) {
ValidateShippingOptionOrPaymentItem(item, item_names, execution_context,
exception_state);
@@ -207,6 +240,11 @@
String& shipping_option_output,
ExecutionContext& execution_context,
ExceptionState& exception_state) {
+ if (input.size() > kMaxListSize) {
+ exception_state.ThrowTypeError("At most 1024 shipping options allowed");
+ return;
+ }
+
HashSet<String> unique_ids;
for (const PaymentShippingOption& option : input) {
ValidateShippingOptionOrPaymentItem(option, "shippingOptions",
@@ -215,6 +253,12 @@
return;
DCHECK(option.hasId());
+ if (option.id().length() > kMaxStringLength) {
+ exception_state.ThrowTypeError(
+ "Shipping option ID cannot be longer than 1024 characters");
+ return;
+ }
+
if (option.id().IsEmpty()) {
execution_context.AddConsoleMessage(ConsoleMessage::Create(
kJSMessageSource, kWarningMessageLevel,
@@ -274,7 +318,20 @@
if (android_pay.hasEnvironment() && android_pay.environment() == "TEST")
output->environment = payments::mojom::blink::AndroidPayEnvironment::TEST;
+ if (android_pay.hasMerchantName() &&
+ android_pay.merchantName().length() > kMaxStringLength) {
+ exception_state.ThrowTypeError(
+ "Android Pay merchant name cannot be longer than 1024 characters");
+ return;
+ }
output->merchant_name = android_pay.merchantName();
+
+ if (android_pay.hasMerchantId() &&
+ android_pay.merchantId().length() > kMaxStringLength) {
+ exception_state.ThrowTypeError(
+ "Android Pay merchant id cannot be longer than 1024 characters");
+ return;
+ }
output->merchant_id = android_pay.merchantId();
// 0 means the merchant did not specify or it was an invalid value
@@ -340,10 +397,27 @@
tokenization.parameters().GetPropertyNames(exception_state);
if (exception_state.HadException())
return;
+ if (keys.size() > kMaxListSize) {
+ exception_state.ThrowTypeError(
+ "At most 1024 tokenization parameters allowed for Android Pay");
+ return;
+ }
String value;
for (const String& key : keys) {
if (!DictionaryHelper::Get(tokenization.parameters(), key, value))
continue;
+ if (key.length() > kMaxStringLength) {
+ exception_state.ThrowTypeError(
+ "Android Pay tokenization parameter key cannot be longer than "
+ "1024 characters");
+ return;
+ }
+ if (value.length() > kMaxStringLength) {
+ exception_state.ThrowTypeError(
+ "Android Pay tokenization parameter value cannot be longer than "
+ "1024 characters");
+ return;
+ }
output->parameters.push_back(
payments::mojom::blink::AndroidPayTokenizationParameter::New());
output->parameters.back()->key = key;
@@ -365,6 +439,12 @@
return;
if (basic_card.hasSupportedNetworks()) {
+ if (basic_card.supportedNetworks().size() > kMaxListSize) {
+ exception_state.ThrowTypeError(
+ "basic-card supportedNetworks cannot be longer than 1024 elements");
+ return;
+ }
+
for (const String& network : basic_card.supportedNetworks()) {
for (size_t i = 0; i < arraysize(kBasicCardNetworks); ++i) {
if (network == kBasicCardNetworks[i].name) {
@@ -378,6 +458,12 @@
if (basic_card.hasSupportedTypes()) {
using ::payments::mojom::blink::BasicCardType;
+ if (basic_card.supportedTypes().size() > kMaxListSize) {
+ exception_state.ThrowTypeError(
+ "basic-card supportedTypes cannot be longer than 1024 elements");
+ return;
+ }
+
const struct {
const BasicCardType code;
const char* const name;
@@ -421,6 +507,13 @@
output->stringified_data =
V8StringToWebCoreString<String>(value, kDoNotExternalize);
+ if (output->stringified_data.length() > kMaxJSONStringLength) {
+ exception_state.ThrowTypeError(
+ "JSON serialization of payment method data should be no longer than "
+ "1048576 characters");
+ return;
+ }
+
// Serialize payment method specific data to be sent to the payment apps. The
// payment apps are responsible for validating and processing their method
// data asynchronously. Do not throw exceptions here.
@@ -452,6 +545,11 @@
Vector<PaymentDetailsModifierPtr>& output,
ExecutionContext& execution_context,
ExceptionState& exception_state) {
+ if (input.size() > kMaxListSize) {
+ exception_state.ThrowTypeError("At most 1024 modifiers allowed");
+ return;
+ }
+
for (const PaymentDetailsModifier& modifier : input) {
output.push_back(payments::mojom::blink::PaymentDetailsModifier::New());
if (modifier.hasTotal()) {
@@ -477,6 +575,21 @@
return;
}
+ if (modifier.supportedMethods().size() > kMaxListSize) {
+ exception_state.ThrowTypeError(
+ "At most 1024 supportedMethods allowed for modifier");
+ return;
+ }
+
+ for (const String& method : modifier.supportedMethods()) {
+ if (method.length() > kMaxStringLength) {
+ exception_state.ThrowTypeError(
+ "Supported method name for identifier cannot be longer than 1024 "
+ "characters");
+ return;
+ }
+ }
+
output.back()->method_data =
payments::mojom::blink::PaymentMethodData::New();
output.back()->method_data->supported_methods = modifier.supportedMethods();
@@ -518,8 +631,6 @@
ValidateAndConvertPaymentDetailsModifiers(
input.modifiers(), output->modifiers, execution_context,
exception_state);
- if (exception_state.HadException())
- return;
}
}
@@ -536,8 +647,6 @@
ValidateAndConvertPaymentDetailsBase(input, output, shipping_option_output,
execution_context, exception_state);
- if (exception_state.HadException())
- return;
}
void ValidateAndConvertPaymentDetailsUpdate(const PaymentDetailsUpdate& input,
@@ -580,6 +689,12 @@
return;
}
+ if (input.size() > kMaxListSize) {
+ exception_state.ThrowTypeError(
+ "At most 1024 payment methods are supported");
+ return;
+ }
+
for (const PaymentMethodData payment_method_data : input) {
if (payment_method_data.supportedMethods().IsEmpty()) {
exception_state.ThrowTypeError(
@@ -588,6 +703,21 @@
return;
}
+ if (payment_method_data.supportedMethods().size() > kMaxListSize) {
+ exception_state.ThrowTypeError(
+ "At most 1024 payment method identifiers are supported");
+ return;
+ }
+
+ for (const String identifier : payment_method_data.supportedMethods()) {
+ if (identifier.length() > kMaxStringLength) {
+ exception_state.ThrowTypeError(
+ "A payment method identifier cannot be longer than 1024 "
+ "characters");
+ return;
+ }
+ }
+
output.push_back(payments::mojom::blink::PaymentMethodData::New());
output.back()->supported_methods = payment_method_data.supportedMethods();
@@ -858,6 +988,11 @@
return;
}
+ if (details.hasId() && details.id().length() > kMaxStringLength) {
+ exception_state.ThrowTypeError("ID cannot be longer than 1024 characters");
+ return;
+ }
+
PaymentDetailsPtr validated_details =
payments::mojom::blink::PaymentDetails::New();
validated_details->id = id_ =
