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

Portals can't exist in auxiliary browsing contexts #216

Open
jakearchibald opened this issue Jun 23, 2020 · 13 comments
Open

Portals can't exist in auxiliary browsing contexts #216

jakearchibald opened this issue Jun 23, 2020 · 13 comments
Labels
spec todo A nitty-gritty detail that needs spec work

Comments

@jakearchibald
Copy link
Collaborator

Right now we prevent portals existing in iframes, but I think we need to do the same for auxiliary browsing contexts.

JS can control the size of these, and if portals inherit the size of the top level window, that creates a communication channel.

I guess we could limit this to cross-origin portals, but maybe that's more confusing?

@jakearchibald jakearchibald added the spec todo A nitty-gritty detail that needs spec work label Jun 23, 2020
@lucasgadani
Copy link
Collaborator

That makes sense. Wouldn't this be equivalent to requiring COOP: same-origin for portals?

@jakearchibald
Copy link
Collaborator Author

Interesting! I don't fully understand COOP yet, but if that prevents the 'parent' origin from resizing the auxiliary window, then yeah that works.

@lucasgadani
Copy link
Collaborator

@jyasskin

@jeremyroman
Copy link
Collaborator

Why is COOP required? Any window opened with noopener (i.e., not auxiliary) should be fine because you don't have a window handle to resize it, no?

@jakearchibald
Copy link
Collaborator Author

Yeah, that's fair. I always forget that noopener impacts more than just window.opener.

@lucasgadani
Copy link
Collaborator

I don't think that's enough -- for example, if a browsing context A opens a browsing context B, both in the same BC unit, then A shouldn't be able to create portals, because B would have access to A's window. (i.e. it is not sufficient to prevent B from creating portals).

@jeremyroman
Copy link
Collaborator

I don't understand how that relates to the sizing-based attack in this issue.

@lucasgadani
Copy link
Collaborator

I'm not totally familiar with the restrictions on window resizing APIs, but in theory, one could resize the window.opener window to establish the communication with the portal.

@jeremyroman
Copy link
Collaborator

Windows which are not auxiliary cannot be resized.

@domenic
Copy link
Collaborator

domenic commented Aug 25, 2020

To clarify something important: a lot of things are auxiliary BCs that you might not think of as popups. For example, <a href="foo" target="_blank"> will open a new tab and that new tab is an auxiliary BC. You can do similar tricks with <form target="_blank">. So if we're over-restrictive here, we're dramatically decreasing the usefulness of portals, e.g. any external page which is linked to from Wikipedia or Twitter can not use portals.

Also, to be honest, I'm not sure whether right-click, open in new tab is an auxiliary BC or not.

So I think the right restriction here to prevent the attack is: no portals in auxiliary BCs that are created by script (as opposed to by the action of the user). Stated positively, you can use portals if you are a top-level BC that is not an auxiliary BC created by script.

I don't think we can further narrow the restriction. That is, if we said you can use portals if you are a top-level BC that either has no opener, or is not an auxiliary BC created by script, then I believe the no-opener case could be used for an attack. You'd do window.open("collaborating-page.html", "_blank", "noopener"), then use BroadcastChannel to tell collaborating-page.html to resize itself, and thus communicate information to the portal embedded in collaborating-page.html.

This "created by script (as opposed to by the action of the user)" appears in the HTML and CSS specs already. I might try to formalize it, although I'd need to figure out how many ways of creating auxiliary browsing contexts by script exist, besides window.open(). Anyway, we can probably use it as-is.

@bokand
Copy link
Contributor

bokand commented Aug 25, 2020

You'd do window.open("collaborating-page.html", "_blank", "noopener"), then use BroadcastChannel to tell collaborating-page.html to resize itself, and thus communicate information to the portal embedded in collaborating-page.html.

Nit: in Chrome, at least, this only causes a new tab which can't be resized; you'd need to specify some windowFeatures like width/height to force a new window to be created. But the point remains.

Note also that popups (both the new window and new tab varieties) require a user gesture so this probably makes it less attractive as a communication channel.

But this seems to me like a fairly niche edge-case - it'd be more straightforward and less restrictive to lock down window.resizeXx rather than browsing context types.

Suggestion: If a portal's embedder browsing context has had any of the window.resize APIs called on it, the portal's internal layout size gets locked to its current value until it's activated. Given that usage of resize APIs should be rare[1], and the only consequence is that your activation might experience minor layout jank, this seems like a fairly minimal restriction.

[1] The chromestatus shows this occurs on ~0.17% of pages - which seems high. If that's too high we could also lock down only on successful requests which I'd guess is much smaller.

@domenic
Copy link
Collaborator

domenic commented Aug 26, 2020

Suggestion: If a portal's embedder browsing context has had any of the window.resize APIs called on it, the portal's internal layout size gets locked to its current value until it's activated. Given that usage of resize APIs should be rare[1], and the only consequence is that your activation might experience minor layout jank, this seems like a fairly minimal restriction.

Oh, yeah, I like this a lot better. We could even un-lock the value if the user resizes the window after the resize* APIs are called.

Does anyone see any holes here? Otherwise I'd be happy to consider this the plan of record, and you can work on updating the explainer/spec in that direction.

@jeremyroman
Copy link
Collaborator

+1; seems very reasonable to say that the portal bc's size doesn't update in response to programmatic resize.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec todo A nitty-gritty detail that needs spec work
Projects
None yet
Development

No branches or pull requests

5 participants