Closed Bug 1566868 Opened 5 years ago Closed 5 years ago

Cross-Origin-Opener-Policy can be used to escape sandboxing

Categories

(Core :: DOM: Networking, defect, P2)

Product:
Core
Component:
DOM: Networking
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla71
Tracking Flags:
Tracking Status
firefox-esr68 --- unaffected
firefox69 --- disabled
firefox70 --- disabled
firefox71 --- fixed

People

(Reporter: annevk, Assigned: CuveeHsu)

References

Details

(Keywords: sec-low, Whiteboard: [necko-triaged])

Attachments

(2 files, 1 obsolete file)

ctx->Top()
2.18 KB, text/plain
Details
Bug 1566868 - network error for non empty sandboxing flag and non-null COOP
47 bytes, text/x-phabricator-request
Details | Review

If you create a sandboxed popup (sandbox needs to be able to create popups) and navigate that to a Cross-Origin-Opener-Policy document the result should be a network error. To ensure that when you use Cross-Origin-Opener-Policy, you can be guaranteed a clean slate.

In https://github.com/web-platform-tests/wpt/pull/17606 look for html/cross-origin-opener-policy/coop-sandbox.https.html.

Given that sandboxing is a security feature, we should fix that before release I think.

Bugbug thinks this bug is a task, but please change it back in case of error.

Type: defect → task
Type: task → defect

Hey Anne, we are not supporting Cross-Origin-Opener-Policy as of now. Did you file by accident, or am I missing something?

Flags: needinfo?(annevk)

We support COOP with pref-off.

Indeed, this is a sandboxing bypass in an important new feature that we need to fix before shipping said feature (hence it blocking the "final" COOP bug).

Flags: needinfo?(annevk)

It seems Bug 1543066 is filed in Networking, hence moving this one over as well.

Component: DOM: Security → DOM: Networking
Whiteboard: [necko-triaged]

I'm wondering if it's a dup of Bug 1555036, though we can't pass html/cross-origin-opener-policy/coop-sandbox.https.html
I can't see SANDBOXED_AUXILIARY_NAVIGATION is set in docshell/httpchannel.

Hello Anne,
(a) I'm confused with the summary of bug: Cross-Origin-Opener-Policy can be used to escape sandboxing
Looks like COOP headers could remove all the sandbox flags after navigation.
However, in html/cross-origin-opener-policy/coop-sandbox.https.html, we're testing the message shouldn't arrive, which indicates a network error.
I believe I misunderstand something (escape indicates network error instead of removal?) since I can't match the summary and the test.

(b)
I guess html/cross-origin-opener-policy/coop-sandbox.https.html is about 5.i in navigating in top-level coop spec

5. If the result of matching currentCOOP, currentOrigin, potentialCOOP, and potentialOrigin is false and one of the following is false

    doc is the initial about:blank document
    currentCOOP's unsafe-allow-outgoing is true
    potentialCOOP is null

then:

    i. If bc's popup sandboxing flag set is not empty, then navigate bc to a network error and abort these steps.

However, it requires that matching algorithm returns false.
In the test, coop-sandbox.https.html open a popup coop-coep.py. Four browsing contexts (top-level and iframe/opener and openee) are with COOP:same-origin and same origin.
Looks like we should not have a network error.

Which step I misinterpreted?

Flags: needinfo?(annevk)

Thanks, I think you're right that the test case is incorrect. Same-origin sandboxed content with allow-same-origin and allow-scripts cannot be sandboxed so letting that escape the sandbox is fine. This mainly wants to protect against A (with COOP) embedding a sandboxed B (with allow-popups and allow-scripts and maybe event allow-same-origin) and that trying to popup B (with COOP) and succeeding (rather than getting a network error). Does that make sense?

Flags: needinfo?(annevk)

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

Thanks, I think you're right that the test case is incorrect. Same-origin sandboxed content with allow-same-origin and allow-scripts cannot be sandboxed so letting that escape the sandbox is fine. This mainly wants to protect against A (with COOP) embedding a sandboxed B (with allow-popups and allow-scripts and maybe event allow-same-origin) and that trying to popup B (with COOP) and succeeding (rather than getting a network error). Does that make sense?

I suppose popup B's opener is the sandboxed iframe B.
It's same origin, so coop match returns true. No network error.

Even if A (with COOP) embedded sandboxed iframe B, and popup C (with COOP) with opener B.
It's cross origin, coop match returns false, so network error occurs.
I thought that's what we want, isn't it?

Flags: needinfo?(annevk)

I think something went wrong with bug 1579012. If A embeds B and B popups B' then B's initial about:blank inherits from B, but the COOP match should be relative to A. It would probably be cleanest if we store the COOP policy on the top-level browsing context.

Needinfo'ing Nika to reaffirm and to help me keep track of the overall design goal here.

(So for these scenarios, for the first there should be a network error as there's no COOP match relative to B's embedder (A) and for the second there should be one as well.)

Flags: needinfo?(annevk) → needinfo?(nika)

I think what you're saying is accurate. In general COOP has no meaning outside of toplevel documents, so when doing comparisons, we should generally always compare against the toplevel.

I haven't quite wrapped my head around what that means for sandboxed popups yet, but I think it'd be reasonable to always network error when loading a sandboxed popup which would otherwise perform a COOP switch, under the assumption that breaking out of the sandbox is bad.

Flags: needinfo?(nika)

(a) For coop matching algorithm, we need to compare not only the top-level parent's COOP but also top-level parents' origin.
(b) Looks like all we need to do for sandbox is Bug 1555036.

A tedious question:
this points out ctx->Canonical()->GetCurrentWindowGlobal()->DocumentPrincipal() is the principal of iframe's parent.

I'm testing a simple iframe (no sandbox).
I still get the origin of iframe instead of the origin of its parent.

GetCurrentEmbedderGlobal comes out a null origin.
Is there any way to get the origin of top-level window?

Flags: needinfo?(nika)

(In reply to Junior [:junior] from comment #13)

Is there any way to get the origin of top-level window?

IIRC this should generally work out (with the proper null principal checks woven in):

ctx->Top()->Canonical()->GetCurrentWindowGlobal()->DocumentPrincipal()
Flags: needinfo?(nika)
Assignee: nobody → juhsu
Status: NEW → ASSIGNED
Attached file ctx->Top() — 
```
nsHttpChannel::HasCrossOriginOpenerPolicyMismatch - doc:1 result:1 - compare:0
doc origin:https://coop-diff-origin.appspot.com/bug/firefox_iframe_B.html - res origin: https://coop-checker.appspot.com/bug/firefox_helper_A.html
ctx 0x1324e6580 ctx->Top() 0x1324e6580
top origin https://coop-diff-origin.appspot.com/bug/firefox_iframe_B.html

The log is from patch comment 15.
Test with https://coop-checker.appspot.com/bug/firefox_main_A.html and clicking button in iframe.

Top level window with origin: https://coop-checker.appspot.com
iframe origin: https://coop-diff-origin.appspot.com

Get get the same browsing context for ctx->Top() in parent process, which seems wrong.
This is after all the windows are created.

Child process looks fine: https://searchfox.org/mozilla-central/rev/deaf58acd27bc663750548e303ac2b461a821cf8/docshell/base/BrowsingContext.cpp#138
It looks not good, right?

Flags: needinfo?(nika)

Per out of band discussion, I parse the wrong context tree.
Looks like we need to cache the top-level opener's origin in browsing context at this moment.

Flags: needinfo?(nika)
Blocks: resab
Pushed by juhsu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e0427b37129b
coop comparison with top-level parents r=valentin

about:blank documents inherit cross-origin opener-policy from their creator's top-level browsing context's active document at the time of creation.
Note: When A embeds B and B creates a popup, this means the popup's initial about:blank document has origin B and A's cross-origin opener-policy.

We have a new spec clarification.

Flags: needinfo?(juhsu)

I made the clarification in comment 22 yesterday, but thanks to Junior I took another look at the problem as I had forgotten about the issues pointed out in comment 7.

I now think html/cross-origin-opener-policy/coop-sandbox.https.html is correct.

If a browsing context has a popup sandboxing flag set, it's a popup and it shouldn't be able to contain anything with a non-null COOP (note that Cross-Origin-Opener-Policy: unsafe-inherit might still work) as that would mean escaping the sandboxing, however mild that sandbox may be (to be clear, this is a reversal of my statement in comment 8).

Then, if a browsing context swap happened due to unsafe-allow-outgoing not being set, the new top-level browsing context still ought to be sandboxed. So we need to copy the popup sandboxing flag set over. (The alternative would be for sandboxed environments governed by COOP without unsafe-allow-outgoing to not be able to open popups at all.)

I have updated the navigation section at the end of https://gist.github.com/annevk/6f2dd8c79c77123f39797f6bdac43f3e to account for this.

(The one other thing about sandboxing that is captured in https://github.com/whatwg/html/issues/4921 was already okay in Firefox if I understood it correctly from Nika. Namely that CSP influences the origin of the document we are navigating to in time. So if you navigate to a document that has an opaque origin due to sandboxing it'll result in a non-match.)

I hope I have now considered all angles appropriately here.

Attachment #9099433 - Attachment is obsolete: true
Pushed by juhsu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/906731e277bb
network error for non empty sandboxing flag and non-null COOP r=valentin

https://hg.mozilla.org/mozilla-central/rev/906731e277bb

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox71: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: