Open Bug 1460425 Opened 6 years ago Updated 2 years ago

Decide on what process the unprivileged PaymentRequest UI should live in

Categories

(Firefox :: WebPayments UI, enhancement, P3)

enhancement

Tracking

()

People

(Reporter: MattN, Unassigned)

References

Details

(Whiteboard: [webpayments-reserve])

For responsiveness and defensive-in-depth against XSS attacks (due to being given untrusted strings from the merchant to be rendered), I implemented the PaymentRequest unprivileged UI in a content process to keep it separate from the process with the full credit cards (and other main process data).

For Spectre/Meltdown mitigations we probably don't want to be in just any process. Should we switch to the parent process or jsut ensure we're in a specific content process? There are performance, UX, and security tradeoffs to consider. Surprisingly, we don't have native dropdown styling when in the parent process :(
Flags: qe-verify-
Flags: needinfo?(ptheriault)
Based on what I know so far, I _think_ we're OK if this is just in a specific content process, but let's definitely flag this for security review and see if pauljt has further insights.
I haven't had a chance to look at code yet but I can offer general comments based on the types of risks we are trying to mitigate. 

Risk 1: Rendering untrusted content
The main reason to use a content process, is because we expect to be rendering untrusted HTML content. My understanding is that this doesn't apply to PaymentRequest currently but might in the future.

Risk 2: Privilege Escalation via script injection
If we are creating UI which involves untrusted strings (as highlighted in comment 0), then doing this in the content process seems like it makes sense. I would hope we can avoid any risk here by avoiding dangerous patterns (innerHTML and friends) but certainly its a good defense in depth control to have this UI in a content process, because it means even if a web page is able to achieve chrome-privileged script injection, this chrome script will be limited in capability due to execution in a child process. Notably, its not possible to access the file system or change preferences directly. However running the payment UI in a content process does complicate the authorisation model (see next risk)

Risk 3: Compromised content process can directly read all credit card data
The PaymentRequest API involves the browser as a trusted mediator to allow the user to grant access to payment card data. What we want to avoid is that in the case that a compromised content process can just reach into the data store an extract credit cards - the user must always be involved in credit card data grants (I assume there isn't a "always grant" scenario). In any case, a decision in the parent process, OR a trusted content content process which the decision is delegated is needed to mediate payment requests.

Those are the main risks I had in mind. Something I would like to understand is "if we have a compromised content process, what is the impact to the payment request API". We might find that we are limited in what we can enforce until spectre/meltdown mitigations are in place (for example, right now, a chrome privileged script in a content process can load ANY origin as far as I know, so it might be difficult to enforce the above requirement). 

An illustrative example is that of webrtc - we mediate the webertc request in the parent, so that if you have never granted access to the camera, a compromised content process can not just request to turn the camera on. However if you have previously granted access to an origin to be able to use the camera, a content process can just navigate to the origin which has access and turn on the camera (the attacker can guess the page the which has access though). Spectre/meltdown changes will improve this, in the parent will be able to know what sites are loaded in the content process and enforce this. 

Anyways, I see you have code up already, so I will file myself a bug to review/understand your current flow. From the first glance I just had it looks fine to me, but I'll need to spend some time to understand it more completely.
Flags: needinfo?(ptheriault)
BTW I'm specifically thinking about bug 1432825 in terms of improving the ability to map principals to content processes.
Paul, you wrote:

> The main reason to use a content process, is because we expect to be rendering untrusted HTML content. My understanding is that this doesn't apply to PaymentRequest currently but might in the future.

As Matthew noted, the PaymentRequest sheet does consume untrusted strings from the payee website (e.g., labels for payment items are defined in the W3C Payment Request API as instances of the Web IDL `DOMString` construct), but does not render untrusted HTML. Although we believe that the Google Chrome browser renders untrusted HTML from third-party payment apps (say, PayPal) in its implementation of the W3C Payment Handler API, we have not yet determined how we would implement the latter API. However, future-proofing seems like a safe choice...
Priority: P2 → P3
Whiteboard: [webpayments] → [webpayments-reserve]
So revisiting this bug, based on my security testing of the current state (that is WIP PaymentUI for basic-card, and not including the Web Payment Request API). The PaymentRequest UI currently lives in a content process - not necessarily the same content process as the page which triggered the PaymentRequest. 

Risk 1: as per previous comment, we don't render any untrusted HTML. I did see cases where overly long strings could break layout, but that likely due to the WIP state of the payment UI. 

Risk 2: not an issue, there is strict input validation, and more importantly we use safe methods for injecting untrusted text into the DOM of the payments page

Risk 3: We take pains to avoid sending the full CC to the content process, until after the user has clicked "PAY". This is great, though one issue is that currently the Payments UI is in a regular content process, along with untrusted web content. To avoid a compromised child process subverting the Payments UI, we need to at least avoid putting the Payments UI in the same process as the web page that launch it. 

So if we want to avoid the risk[1] of a child compromise being able to read credit cards, we need to ensure that the payments UI gets a content process separate to regular web content. The best candidate is likely the new "Privilege Content Process" introduced for Activity Stream in bug 1469072. We may want a different choice for payment handler(we wouldn't want untrusted HTML in the privileged process), or maybe by then we will have more progress towards site-isolation which removes the risk. 


[1] I don't know what the process model looks like for existing form fill and password fill code looks like. Hopefully these follow the same model that user mediation (in the form of UI in the parent) is required to mediate access to the filling form or password data.
Depends on: 1469072
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.