1508510 - Calling r.abort() in a payment event handler cause IPDL errors, killing the child process

Status: NEW

(Core :: DOM: Web Payments, defect, P3)

Description

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Attachment: command line logs

Hard to explain this since I only vaguely understand it but here goes: Payment Request handlers can take a promise instead of a function. Passing a function _call_ to a function which calls r.abort() AND also returns a value (like "error") causes the tab to crash. See p[1] which just does:

function badHandler(r) {
  r.abort();
  return { error: "" }
}

function crash() {
  let request = new PaymentRequest(method, details, options);
  request.onshippingoptionchange = ev => ev.updateWith(badHandler(request));
  request.show()
}

This results in the "Gah. Your tab just crashed." message. I'm not sure if its IPC related or not, but attached is the console logs from ASAN nightly. 

I don't know if this is a security related crash or not (i think not, given that its just a tab crash), but this API isn't enabled yet so I don't see the need to restrict it. 

[1] https://misuse.co/t/pay/paycrash.html

Comment 1

[Paul Theriault [:pauljt] (no longer reading bugmail)]

If it helps I think we are hitting this line:
https://searchfox.org/mozilla-central/source/__GENERATED__/ipc/ipdl/PPaymentRequestParent.cpp#240

Comment 2

[Paul Theriault [:pauljt] (no longer reading bugmail)]

I think I figured this out. I think request.abort() results ultimately ends up removing the payment request from PaymentRequestService->mRequestQueue at [1]. Then, the child sends the PaymentUpdateActionRequest (as a result of event.updateWith()) this line [2] in PaymentRequestParent fails .

PPaymentRequestParent::OnMessageReceived[3] then passes a MsgProcessingError up to MessageChannel.cpp[4], which then calls ContentParent::ProcessingError[5], which does KillHard(aReason).



[1] https://searchfox.org/mozilla-central/rev/b03a62c3c82316e733a3b09622c1cb7e59f64cc3/dom/payments/PaymentRequestService.cpp#411

[2] https://searchfox.org/mozilla-central/rev/b03a62c3c82316e733a3b09622c1cb7e59f64cc3/dom/payments/ipc/PaymentRequestParent.cpp#83

[3] https://searchfox.org/mozilla-central/source/__GENERATED__/ipc/ipdl/PPaymentRequestParent.cpp#202

[4] https://searchfox.org/mozilla-central/rev/b03a62c3c82316e733a3b09622c1cb7e59f64cc3/ipc/glue/MessageChannel.cpp#2591

[5] https://searchfox.org/mozilla-central/rev/b03a62c3c82316e733a3b09622c1cb7e59f64cc3/dom/ipc/ContentParent.cpp#1657

Comment 3

[Matthew N. [:MattN]]

Yaron, can you leave the triage of this component to Eden/Marcos since we have a system for it: https://wiki.mozilla.org/Firefox/Features/Web_Payments/DOM

Comment 4

[Yaron Tausky [:ytausky]]

Sure, I'll remove you from my triage query. Sorry for the interruption. :-)

Comment 5

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Dunno why bugzilla clipped the links above. The first link is 
https://searchfox.org/mozilla-central/source/dom/payments/PaymentRequestService.cpp#411

or permalink: 

https://searchfox.org/mozilla-central/rev/b03a62c3c82316e733a3b09622c1cb7e59f64cc3/dom/payments/PaymentRequestService.cpp#411

Comment 6

[Kohei Yoshino]

Sorry for the garbled links! Will be fixed tomorrow.

Comment 7

[Eden Chuang[:edenchuang]]

Attachment: Ignoring PaymentRequestUpdateEvent::updateWith while PaymentRequest::Abort is called.

Ignoring the merchant update, while the PaymentRequest is aborting.

Comment 8

[Eden Chuang[:edenchuang]]

Attachment: mochitest for ignoring updateWith

Comment 9

[Release mgmt bot [:suhaib / :marco/ :calixte]]

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:edenchuang, could you have a look please?

Comment 10

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Just a thought - is this IPC mechansim exposed even though the API isn't enabled?

Comment 11

[Eden Chuang[:edenchuang]]

Unfortunately, yes. But the PayementRequest API is the only one entry point for this IPC.