1570889 - blob URLs and CSP sandbox'ed pages should inherit Cross-Origin-Opener-Policy

Status: NEW

(Core :: DOM: File, enhancement, P3)

Description

[Frederik Braun [:freddy]]

A blob URL opened from a page with COOP enabled is not protected by COOP. I think it should inherit the security header.

Comment 1

[Frederik Braun [:freddy]]

Jun's PoC is at http://vuln.shhnjk.com/COOP_bypass.php

Comment 2

[Anne (:annevk)]

1. As this isn't enabled by default no need to have this bug restricted.
2. It's not clear what the desirable design here is. As it stands the blob URL document is isolated from its opener (at least, I think so, window.opener returns null), but not from popups it itself opens. Do you or Jun suggest it should not be isolated from its opener and should be isolated from popups it opens (if unsafe-allow-outgoing is not set)?

Comment 3

[Frederik Braun [:freddy]]

This is not about the blob being isolated from its opener, but anything opened from the blob.
I don't think the blob needs to be isolated from its opener (they are same-origin after all, aren't they?) but the blob's popup shouldn't be able to navigate the blob. As I said in comment 0: I think the blob document should inherit the COOP from it's opener.

Comment 4

[Anne (:annevk)]

In comment 2, point 2 is incorrect. The blob URL popup's opener is only null because of target=_blank. I tried to recreate this bypass in an automated test, but couldn't, we pass that one: https://github.com/web-platform-tests/wpt/pull/19949.

PoC is at https://vuln.shhnjk.com/COOP_bypass.php now that we require HTTPS, but it still works. Is there a difference for user-opened popups somehow?

Comment 5

[Junior [inactive]]

IIRC :nika has some idea in mind by letting document channel switch process for different protocols.
Would you mind to share hints on top of your head? Thanks.

Comment 6

[Nika Layzell [:nika] (ni? for response)]

Currently, the only place loads are checked for COOP-related process switches is within onMayChangeProcess. This API is only called for loads which go through DocumentChannel, which is a newer API allowing process switching decisions to be made by the parent process, in response to the server's response headers & final URL.

We currently use DocumentChannel for all loads where the first URL in the chain is http(s), due to the check here: https://searchfox.org/mozilla-central/rev/ce02064d8afc8673cef83c92896ee873bd35e7ae/docshell/base/nsDocShell.cpp#9608-9609. There is some desire to use this new process switching mechanism for more types of loads, to unify the places where we can trigger process switches. This is currently being tracked in bug 1575120.

Process switches for non-http loads are currently performed by the checks in E10SUtils.shouldLoadURI: https://searchfox.org/mozilla-central/rev/ce02064d8afc8673cef83c92896ee873bd35e7ae/toolkit/modules/E10SUtils.jsm#710-790. These checks are performed before the navigation even starts. This works OK for simple loads, as no redirects can happen after the method is called.

If bug 1575120 isn't practical short-term, we may need to do a hacky solution using the shouldLoadURI mechanism. If the current document has COOP set, and the new URI has no way of having COOP set (e.g. it's a blob URI), we'd prevent the loads from occuring by returning false, and force a process switch. We'd likely also need to add checks in the parent to ensure the load doesn't end up in a COOP+COEP process.

Comment 7

[Junior [inactive]]

Quick note:
It's weird that we can get document COOP in anne's case but not the PoC

anne's wpt
2019-11-06 23:18:55.333341 UTC - [Parent 35641: Main Thread]: D/nsHttp nsHttpChannel::HasCrossOriginOpenerPolicyMismatch - doc:1 result:0 - compare:0
2019-11-06 23:18:55.333392 UTC - [Parent 35641: Main Thread]: D/nsHttp doc origin:https://web-platform.test:8443/html/cross-origin-opener-policy/blob-popup.https.html - res origin: https://www1.web-platform.test:8443/html/cross-origin-opener-policy/resources/coop-coep.py?coop=x&coep=x&channel=78c17a82-f46a-414e-855f-37dd5b3782f2

PoC
2019-11-06 23:23:42.523832 UTC - [Parent 35641: Main Thread]: D/nsHttp nsHttpChannel::HasCrossOriginOpenerPolicyMismatch - doc:0 result:0 - compare:1
2019-11-06 23:23:42.523840 UTC - [Parent 35641: Main Thread]: D/nsHttp doc origin:https://vuln.shhnjk.com/COOP_bypass.php - res origin: https://attack.shhnjk.com/copen.html

And I hit this in Nightly debug for anne's wpt
Nightly debug
JavaScript error: resource:///modules/sessionstore/SessionStore.jsm, line 2710: NS_ERROR_NOT_AVAILABLE: Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIProcessSwitchRequestor.crossOriginOpenerPolicy]

Maybe bug 1594166 could ease this at the moment?

Comment 8

[Junior [inactive]]

Comment 1 scenario in current Nightly:

Step 1:
https with COOP open a blob popup: process switch, but COOP is not set in the new browsing context.

Step 2:
the blob document open another https popup: no process switch, so there's an opener.

Therefore, we might need another approach from comment 6 which doesn't switch process in step 2.

Comment 9

[Junior [inactive]]

Anne, I'm confused what we should change here.
For the case in Comment 1,
HTTPS with COOP (origin A) open a blob popup
=> no headers in the blob popup
=> the potential COOP is null
=> COOP mismatch, create a new browsing context (process)

(Looked at the note under $3.3.3, the blob is in secure context, right?)

However, I don't see any COOP setup in the new browsing context (newTLBC) in spec
Therefore, looks like we get a clean plate in the blob popup. The new popup (origin B) and its opener (blob) are with null COOP.
Allow the opener access owning to COOP match.

Hence, I guess we should change spec if we want to do the COOP sandboxing.

Comment 10

[Anne (:annevk)]

Yeah, I think the suggestion is that a blob URL would inherit the COOPness of its creator. That would indeed require a specification change and as inheritance is tricky probably needs careful consideration. Given that our current behavior is safe (a blob URL ends up in its own browsing context group) this does not seem like a high priority to me, especially as blob URL popups are rare.

Does that make sense to everyone?

Comment 11

[Junior [inactive]]

Okay, looks like we can wait for spec change :)

Comment 12

[Anne (:annevk)]

https://vuln.shhnjk.com/COOP_bypass.php is affected by bug 1522083 I think. Currently the blob: URL popup this creates cannot be interacted with at all. And also, comment 10 forgets that we have some inheritance already as per comment 4.

Maybe the main difference/problem here is that <a> is used? I'll see about creating more tests. (If I change target=_blank to target=test I can no longer reproduce the attack btw. It seems COOP inheritance is now working correctly here.)

Comment 13

[Anne (:annevk)]

https://github.com/web-platform-tests/wpt/pull/19947 now tests this. The problem is in particular with rel=noopener on the blob URL link. Other aspects of blob URLs appear fine.

Comment 14

[Anne (:annevk)]

(To be clear, this still blocks resab, just no longer directly.)

Comment 15

[Junior [inactive]]

The TIMEOUT with rel=noopener is owing to postMessage doesn't work with null opener.
https://searchfox.org/mozilla-central/rev/61f224ec08ddc6f9a93ac45c8c3c5f7159be7c2a/testing/web-platform/tests/html/cross-origin-embedder-policy/resources/script-factory.js#21
It shows line 18: TypeError: can't access property "postMessage", window.opener is null

I've tested with no COOP/COEP header and other browser as well, and this statement stands.
I'm not sure if we have ways to communicate with the opener with rel=noopener, but this test should be removed or fixed.

What do you think, Anne?

Comment 16

[Anne (:annevk)]

https://github.com/web-platform-tests/wpt/pull/22411 has a fix for the tests. But the bug still shows in Jun's test (comment 4) so I guess it's really about opening a cross-origin popup from a blob URL that's supposed to have "inherited" COOP.

Creating an automated test for that is somewhat harder, but still doable. If the cross-origin popup that's not supposed to have an opener has a frame that's same-origin with the blob and the blob's opener it could communicate via BroadcastChannel through there. Or we could stash state on the server.

Is that something you could work on as part of fixing this?

Comment 17

[Junior [inactive]]

(In reply to Anne (:annevk) from comment #16)

https://github.com/web-platform-tests/wpt/pull/22411 has a fix for the tests. But the bug still shows in Jun's test (comment 4) so I guess it's really about opening a cross-origin popup from a blob URL that's supposed to have "inherited" COOP.

Per comment 12, it should be affected by bug 1522083. The blob popup is totally blocked by pref dom.targetBlankNoOpener.enabled.
However, if there's a manual/automated test, I could work on what comment 6 says. (wpt/22411?)

Creating an automated test for that is somewhat harder, but still doable. If the cross-origin popup that's not supposed to have an opener has a frame that's same-origin with the blob and the blob's opener it could communicate via BroadcastChannel through there. Or we could stash state on the server.

Is that something you could work on as part of fixing this?

Comment 18

[Anne (:annevk)]

I could have sworn I could reproduce the test from comment 4, but now I can't again. (And if I change target=_blank to target=x or add rel=opener the final popup does not have an opener.) So yeah, it does look like bug 1522083 is the only problem here, unless someone can point out what I'm missing.

Comment 19

[Anne (:annevk)]

Okay, now the issue with blob URLs is resolved I can reproduce comment 4 once again and comment 10 applies.

This bug is similar to bug 1626566. Subhamoy, you and Andrea might want to look into this class of issues to see how to solve it more generically so we don't hit it for each new policy and do the right thing for existing policies. This is tracked specification-wise by https://github.com/w3c/FileAPI/issues/142 and https://github.com/whatwg/html/issues/5198.

Given all that, we'll consider this as follow-up, but it's not going to block shipping of COOP.

(Chrome has the same bug.)

Comment 20

[Jens Stutte [:jstutte]]

Hi Daniel, is the sec-moderate classification (still) appropriate here?

Comment 21

[Jens Stutte [:jstutte]]

As of comment 19 this looks like an enhancement request more than a defect.

Comment 22

[BugBot [:suhaib / :marco/ :calixte]]

Clear a needinfo that is pending on an inactive user.

Inactive users most likely will not respond; if the missing information is essential and cannot be collected another way, the bug maybe should be closed as INCOMPLETE.

For more information, please visit auto_nag documentation.

Comment 23

[Frederik Braun [:freddy]]

This bug is not incomplete and I don't think this should be an enhancement request either.
It used to be a spec issue in how COOP is designed. But with https://github.com/web-platform-tests/wpt/pull/19947 and https://github.com/web-platform-tests/wpt/pull/22411 resolved, it seems this is now a defect in our implementation of COOP.

Can you please reevaluate?

Comment 24

[Jens Stutte [:jstutte]]

Eden is probably a better match here to reevaluate this, thanks.

Comment 25

[Eden Chuang[:edenchuang]]

I will take a look late this week.