From aab330d18c86776b960a09ec324e1d1d5a008187 Mon Sep 17 00:00:00 2001
From: "aestes@apple.com"
 <aestes@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Fri, 9 Feb 2018 19:07:28 +0000
Subject: [PATCH] [Payment Request] Crash in PaymentRequest::canMakePayment()
 when Apple Pay payment method data is missing required fields
 https://bugs.webkit.org/show_bug.cgi?id=182631

Reviewed by Mark Lam.

Source/WebCore:

PaymentRequest::canMakePayment() needs to parse each payment method's serialized data to
determine if it is a supported payment method. If parsing fails by raising an exception, we
intend to skip over that payment method and try the next one. If all payment method data
fail to parse, we resolve the returned promise with false. At no point do we intend to
propagate the parsing exception up to the calling script, however.

Even though we intend to swallow any exceptions from parsing, we failed to clear the
JavaScript VM's exception state. The next time WebCore tries to execute JavaScript, a
release assertion is raised due to seeing an unexpected exception in the VM.

Fix this by using a CatchScope in PaymentRequest::canMakePayment(), and calling
CatchScope::clearException() in the places we intend to swallow exceptions.

Added a test case to http/tests/paymentrequest/payment-request-canmakepayment-method.https.html.

* Modules/paymentrequest/PaymentRequest.cpp:
(WebCore::PaymentRequest::canMakePayment):

LayoutTests:

* http/tests/paymentrequest/payment-request-canmakepayment-method.https-expected.txt:
* http/tests/paymentrequest/payment-request-canmakepayment-method.https.html:


git-svn-id: https://svn.webkit.org/repository/webkit/trunk@228331 268f45cc-cd09-0410-ab3c-d52691b4dbfc
---
 LayoutTests/ChangeLog                              | 10 +++++++++
 ...equest-canmakepayment-method.https-expected.txt |  1 +
 ...ayment-request-canmakepayment-method.https.html | 12 ++++++++++-
 Source/WebCore/ChangeLog                           | 25 ++++++++++++++++++++++
 .../Modules/paymentrequest/PaymentRequest.cpp      | 11 ++++++++--
 5 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 97c96a0..979db65 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2018-02-09  Andy Estes  <aestes@apple.com>
+
+        [Payment Request] Crash in PaymentRequest::canMakePayment() when Apple Pay payment method data is missing required fields
+        https://bugs.webkit.org/show_bug.cgi?id=182631
+
+        Reviewed by Mark Lam.
+
+        * http/tests/paymentrequest/payment-request-canmakepayment-method.https-expected.txt:
+        * http/tests/paymentrequest/payment-request-canmakepayment-method.https.html:
+
 2018-02-09  Ryan Haddad  <ryanhaddad@apple.com>
 
         Update TestExpectations for fast/forms/textarea/textarea-state-restore.html
diff --git a/LayoutTests/http/tests/paymentrequest/payment-request-canmakepayment-method.https-expected.txt b/LayoutTests/http/tests/paymentrequest/payment-request-canmakepayment-method.https-expected.txt
index 9001416..df6fb85 100644
--- a/LayoutTests/http/tests/paymentrequest/payment-request-canmakepayment-method.https-expected.txt
+++ b/LayoutTests/http/tests/paymentrequest/payment-request-canmakepayment-method.https-expected.txt
@@ -3,6 +3,7 @@ PASS If request.[[state]] is "created", then return a promise that resolves to t
 PASS If request.[[state]] is "interactive", then return a promise rejected with an "InvalidStateError" DOMException. 
 PASS If request.[[state]] is "closed", then return a promise rejected with an "InvalidStateError" DOMException. 
 PASS If payment method identifier and serialized parts are supported, resolve promise with true. 
+PASS If a payment method identifier is supported but its serialized parts are not, resolve promise with false. 
 PASS If payment method identifier is unknown, resolve promise with false. 
 PASS Optionally, at the user agent's discretion, return a promise rejected with a "NotAllowedError" DOMException. 
 
diff --git a/LayoutTests/http/tests/paymentrequest/payment-request-canmakepayment-method.https.html b/LayoutTests/http/tests/paymentrequest/payment-request-canmakepayment-method.https.html
index 74791c4..6f4e887 100644
--- a/LayoutTests/http/tests/paymentrequest/payment-request-canmakepayment-method.https.html
+++ b/LayoutTests/http/tests/paymentrequest/payment-request-canmakepayment-method.https.html
@@ -10,7 +10,7 @@
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
 <script>
-    const applePay = Object.freeze({
+const applePay = Object.freeze({
     supportedMethods: "https://apple.com/apple-pay",
     data: {
         version: 2,
@@ -20,6 +20,11 @@
         countryCode: "US",
     }
 });
+const invalidApplePay = Object.freeze({
+    supportedMethods: "https://apple.com/apple-pay",
+    data: {
+    }
+});
 const defaultMethods = Object.freeze([applePay]);
 const defaultDetails = Object.freeze({
   total: {
@@ -98,6 +103,11 @@ promise_test(async t => {
 }, `If payment method identifier and serialized parts are supported, resolve promise with true.`);
 
 promise_test(async t => {
+  const request = new PaymentRequest([invalidApplePay], defaultDetails);
+  assert_false(await request.canMakePayment(), "Apple Pay with invalid data should not be supported");
+}, `If a payment method identifier is supported but its serialized parts are not, resolve promise with false.`);
+
+promise_test(async t => {
   const unsupportedMethods = [
     "this-is-not-supported",
     "https://not.supported",
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 0a5c65d..c533af7 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,28 @@
+2018-02-09  Andy Estes  <aestes@apple.com>
+
+        [Payment Request] Crash in PaymentRequest::canMakePayment() when Apple Pay payment method data is missing required fields
+        https://bugs.webkit.org/show_bug.cgi?id=182631
+
+        Reviewed by Mark Lam.
+
+        PaymentRequest::canMakePayment() needs to parse each payment method's serialized data to
+        determine if it is a supported payment method. If parsing fails by raising an exception, we
+        intend to skip over that payment method and try the next one. If all payment method data
+        fail to parse, we resolve the returned promise with false. At no point do we intend to
+        propagate the parsing exception up to the calling script, however.
+
+        Even though we intend to swallow any exceptions from parsing, we failed to clear the
+        JavaScript VM's exception state. The next time WebCore tries to execute JavaScript, a
+        release assertion is raised due to seeing an unexpected exception in the VM.
+
+        Fix this by using a CatchScope in PaymentRequest::canMakePayment(), and calling
+        CatchScope::clearException() in the places we intend to swallow exceptions.
+
+        Added a test case to http/tests/paymentrequest/payment-request-canmakepayment-method.https.html.
+
+        * Modules/paymentrequest/PaymentRequest.cpp:
+        (WebCore::PaymentRequest::canMakePayment):
+
 2018-02-09  Zalan Bujtas  <zalan@apple.com>
 
         [RenderTreeBuilder] Move multicolumn descendant/sibling removal logic to RenderTreeBuilder
diff --git a/Source/WebCore/Modules/paymentrequest/PaymentRequest.cpp b/Source/WebCore/Modules/paymentrequest/PaymentRequest.cpp
index a06176d..f11178f 100644
--- a/Source/WebCore/Modules/paymentrequest/PaymentRequest.cpp
+++ b/Source/WebCore/Modules/paymentrequest/PaymentRequest.cpp
@@ -489,18 +489,25 @@ void PaymentRequest::canMakePayment(Document& document, CanMakePaymentPromise&&
         return;
     }
 
+    auto scope = DECLARE_CATCH_SCOPE(document.execState()->vm());
     for (auto& paymentMethod : m_serializedMethodData) {
         auto data = parse(document, paymentMethod.serializedData);
-        if (data.hasException())
+        ASSERT(!!scope.exception() == data.hasException());
+        if (data.hasException()) {
+            scope.clearException();
             continue;
+        }
 
         auto handler = PaymentHandler::create(document, *this, paymentMethod.identifier);
         if (!handler)
             continue;
 
         auto exception = handler->convertData(data.releaseReturnValue());
-        if (exception.hasException())
+        ASSERT(!!scope.exception() == exception.hasException());
+        if (exception.hasException()) {
+            scope.clearException();
             continue;
+        }
 
         handler->canMakePayment([promise = WTFMove(promise)](bool canMakePayment) mutable {
             promise.resolve(canMakePayment);
-- 
1.8.3.1