Open Bug 1508510 Opened 10 months ago Updated 6 days ago

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

Categories

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

65 Branch
defect

Tracking

()

People

(Reporter: pauljt, Assigned: edenchuang)

References

Details

Attachments

(3 files)

Attached file 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
Component: Security: Review Requests → DOM: Web Payments
Product: Firefox → Core
Version: unspecified → 65 Branch
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
Priority: -- → P3
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
Flags: needinfo?(ytausky)
Sure, I'll remove you from my triage query. Sorry for the interruption. :-)
Flags: needinfo?(ytausky)
Sorry for the garbled links! Will be fixed tomorrow.
Blocks: 1463025
No longer blocks: 1366652
Ignoring the merchant update, while the PaymentRequest is aborting.
Assignee: nobody → echuang
Attachment #9031198 - Flags: review?(amarchesini)
Attachment #9031199 - Flags: review?(amarchesini)
Attachment #9031198 - Flags: review?(amarchesini) → review+
Attachment #9031199 - Flags: review?(amarchesini) → review+

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?

Flags: needinfo?(echuang)
Flags: needinfo?(echuang)
You need to log in before you can comment on or make changes to this bug.