Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable Payment Request API in CSP/iframe sandbox #698

Closed
shhnjk opened this issue Mar 28, 2018 · 19 comments · Fixed by #822
Closed

Disable Payment Request API in CSP/iframe sandbox #698

shhnjk opened this issue Mar 28, 2018 · 19 comments · Fixed by #822

Comments

@shhnjk
Copy link
Member

shhnjk commented Mar 28, 2018

CSP/iframe sandbox is meant to host untrusted content (by locking down privildge of the untrusted
content). And powerful APIs such as Service Worker, AppCache, etc are not callable from sandboxed contents. For the same reason, Payment Request API should be disabled in sandxboxed content. If anyone feels that there's a valid use case of Payment Request API in sandbox, then it should be only allowed with "allow-same-origin" keyword (Though I don't think there is such a use case).

@rsolomakhin
Copy link
Collaborator

Example of a CSP HTML header that @shhnjk proposes should prevent web payments:

Content-Security-Policy: sandbox allow-scripts

@ianbjacobs
Copy link
Collaborator

@rsolomakhin
Copy link
Collaborator

Do any of the implementers think it would be a good idea to have this in the spec?

@marcoscaceres
Copy link
Member

marcoscaceres commented Mar 29, 2018

Looking quickly, seems we would need to add it to:

And we would need something like this in the PaymentRequest constructor:

If the document object's active sandboxing flag set has the sandboxed payment request browsing context flag set, throw a SecurityError DOMException.

@domenic, does the above seem right? Or is there more to it?

@marcoscaceres
Copy link
Member

marcoscaceres commented Mar 29, 2018

Noting we have allowpaymentrequest... so we would need to work out how they interact.

<iframe sandbox allowpaymentrequest></iframe>

I guess sandbox there bans everything, and allowpaymentrequest enables.... but because PR API is disabled by default cross-origin, we might not need our own flag?

@domenic
Copy link
Collaborator

domenic commented Apr 2, 2018

This is interesting... usually we do not add features to both allow*/feature policy and iframe sandboxing. But, the fact that for top-level pages payment request is on by default, whereas in iframes it is off by default, makes this complicated.

Paging @clelland, our feature policy expert, for this thoughts.

@clelland
Copy link

clelland commented Apr 3, 2018

PaymentRequest should be disabled, by default, in any sandboxed frame, just by virtue of it being cross-origin to the main document. The only way around that would be with something like

    <iframe sandbox="allow-same-origin allow-scripts" srcdoc="..."></iframe>

which would inherit the origin of the parent, and therefore not be cross-origin. That case should be treated like any other same-origin embed, practically, though. (The HTML spec specifically warns against doing that and thinking that it's anything like actually being secure)

allowpaymentrequest, and the equivalent payment policy-controlled-feature will currently enable the API in any cross-origin frame, including a sandboxed one, but that is entirely at the control of the developer including the sandboxed page. I'm pretty confident that that is sufficient to stop any content that shouldn't be using the API from using it, while giving developers enough control to enable it in a sandbox if they need it turned on for any reason.

@shhnjk
Copy link
Member Author

shhnjk commented Apr 28, 2018

Real world example: dropbox.com/enterprise runs with CMS which they made isolation with main dropbox.com using CSP sandbox. XSS in CMS theoretically has no impact on main dropbox.com but in this case, attacker can ask for payment :)
CC: @devd

@clelland
Copy link

clelland commented May 4, 2018

So, to disable paymentrequest completely in any frame, whether same- or cross- origin, sandboxed or not, it is possible to use allow="payment 'none'".

Then, even in the least-sandboxed sandbox:

<iframe allow="payment 'none'" sandbox="allow-same-origin allow-scripts" srcdoc="..."></iframe>

the PaymentRequest API will be denied.

That won't stop a malicious script from reaching up into parent and forcing it to call the API, (or removing the attribute, or even the sandbox attribute, and reloading itself,) but if you're same-origin with your parent, then you've already given up on a lot of origin-based security protections.

Is dropbox.com/enterprise using allow-same-origin and allow-scripts? That's the only way that it should be able to request payment in a sandbox (but has all of the other problems above).

@shhnjk
Copy link
Member Author

shhnjk commented May 5, 2018

dropbox.com/enterprise is using following CSP sandbox.
content-security-policy: sandbox allow-forms allow-scripts allow-top-navigation allow-popups;

FYI to the spec editors (of all kinds), you all are doing great job in restricting cross-origin frames or insecure context for powerful APIs. But in my experience, CSP/iframe sandbox is usually left to implementors and I don't think it's a good idea (especially, now we have major website taking advantage of sandbox). This was also a spec issue in Web App Manifest. And what about Credential Management API?

So I appreciate if spec editors can keep in mind about sandbox and restrict powerful APIs in sandboxed content as you do for cross-origin frames and insecure context. Thanks.

@devd
Copy link

devd commented May 6, 2018 via email

@shhnjk
Copy link
Member Author

shhnjk commented May 6, 2018

Credit card information is saved in a browser and it isn’t tied to any origin. So if user trusts “dropbox.com/enterprise”, that’s a good way to monetize an XSS inside sandbox.

@clelland
Copy link

clelland commented May 7, 2018

dropbox.com/enterprise is using following CSP sandbox.
content-security-policy: sandbox allow-forms allow-scripts allow-top-navigation allow-popups;

That should be sufficient to completely restrict the use of the PaymentRequest API in that frame. That policy should ensure that the frame has an opaque origin, which will be cross-origin with the frame in which it is embedded, and as long as the containing page hasn't explicitly granted access, it will be denied by default. Any other behavior is a bug.

shouldn't the spec rely on current origin of page when making decisions and if it
did, shouldn't it just work out?

Exactly, that is what should happen in this situation.

@clelland
Copy link

clelland commented May 7, 2018

After re-reading all of the comments in this thread, I think I see the issue more clearly. This isn't about iframe sandboxing at all -- that works exactly as intended, as @devd points out. This comes up when the top-level document has sandboxed itself through the CSP sandbox directive. In that case, the browser's default policy is to allow the API in the main frame.

For now, this can be mitigated by also using the header

Feature-Policy: payment 'none'

but perhaps sandboxed main frames should have features like this disabled by default. I don't think it should be a behavior specific to this API, but possibly to Feature Policy generally, for any features which are disabled in cross-origin frames by default.

@devd
Copy link

devd commented May 7, 2018

sounds good. Just to sanity check: is feature-policy support a requirement of payment-request support?

@ianbjacobs
Copy link
Collaborator

I am not familiar with Feature Policy. However, the comment from @clelland that for security reasons powerful features should be disabled by default makes sense to me.

@devd, right now I don't see in the normative dependencies of Payment Request any dependency on feature-policy.

@stpeter
Copy link

stpeter commented May 7, 2018

Regarding Feature Policy, see #600 (it's marked as a future feature).

@shhnjk
Copy link
Member Author

shhnjk commented May 7, 2018

Just to clarify, why I included iframe in the title is because pop ups opened from iframe sandbox would also be a top-level sandboxed document (assuming the escape sandbox flag is not set).

@marcoscaceres
Copy link
Member

Rereading the thread, it seems this will be handled by generically by the Feature Policy spec. I'm going to close this out via: #822

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants