WICG / portals

A proposal for enabling seamless navigations between sites or pages
https://wicg.github.io/portals/
Other
947 stars 66 forks source link

Portals can't exist in auxiliary browsing contexts #216

Open jakearchibald opened 4 years ago

jakearchibald commented 4 years ago

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?

lucasgadani commented 4 years ago

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

jakearchibald commented 4 years ago

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 commented 4 years ago

@jyasskin

jeremyroman commented 4 years ago

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 commented 4 years ago

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

lucasgadani commented 4 years ago

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 commented 4 years ago

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

lucasgadani commented 4 years ago

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 commented 4 years ago

Windows which are not auxiliary cannot be resized.

domenic commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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