1552541 - (CVE-2019-11711) Inner window reuse does not play nicely with document.domain

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Consider this testcase:

<script>
  document.domain = document.domain;
  var e;
  function doIt() {
    var win = window.open("newtab.html");
    win.hello = 5;
    e = win.eval;
  }

  function proceed() {
    e("document.body.innerHTML += 'hey'");
  }
</script>
<button type="button" onclick="doIt()">Open a new tab</button>
<button type="button" onclick="proceed()">Inject script</button>

where newtab.html looks like this:

<script>
  document.write(window.hello);
</script>

Per spec, and in Gecko, if you click the "Open a new tab" button, wait for the new page to load, switch back to the testcase, and then press the "Inject script" button, you will see the text "5 hey" in the new tab.

What's going on here is that inner window reuse does not consider document.domain (see https://html.spec.whatwg.org/multipage/browsing-the-web.html#initialise-the-document-object step 1), so we end up reusing the inner window here. In Gecko, you can get the same effect by inserting a new iframe to a same-origin URL and immediately grabbing .contentWindow.Function; after that you can run any script you want in the new page that loads, even though it has not set document.domain.

What this means is that if you have a page at a.example.com and a page at b.example.com that both set domain to example.com, now the b.example.com page can inject script into any a.example.com page by doing the above!

This is a longstanding problem in Gecko, even back when we revoked access on document.domain changes, because we don't remap wrappers on inner window reuse. It basically dates back to bug 956382, where stopped considering domain for inner window reuse purposes.

Before Firefox 67, any objects you pulled out of the page you're attacking would get opaque wrappers (which caused problems like bug 1551272, actually, in non-attack scenarious, since you'd get an opaque wrapper for win.postMessage), but you can still inject script and exfiltrate data via sending it to servers, of course. Or just inject script that sets document.domain and then you have full access.

In 67, even the opaque-wrapper thing is gone; remapping wrappers on inner window reuse would not help there.

I just did a bit of testing, and as far as I can tell, Chrome and Safari reuse the inner in a lot fewer cases than us. They don't seem to do it for dynamically injected (not parser-created) subframes, and more importantly they don't seem to do it in the testcase above (window.hello ends up undefined in newtab.html). They do reuse if I take out the document.domain = document.domain line.

So maybe we should just make WouldReuseInnerWindow() do EqualsConsideringDomain() (which means never reuse if document.domain was set)?

Comment 1

[Boris Zbarsky [:bzbarsky]]

Credit for finding this should go to [e-mail], by the way.

Comment 2

[Boris Zbarsky [:bzbarsky]]

So I dug into Chrome's code a bit to make sure I understand their behavior. What they have is that https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/loader/document_loader.cc?l=1478-1479&rcl=76fe5b8f86a65179352608bd6bcfa9c0d0297326 calls https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/loader/frame_loader.cc?l=1396&rcl=76fe5b8f86a65179352608bd6bcfa9c0d0297326 and passes it the new document's origin. That then calls https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/weborigin/security_origin.h?l=147&rcl=76fe5b8f86a65179352608bd6bcfa9c0d0297326 on the old (about:blank) document's origin, which does a document.domain-aware check. So they in fact do not end up reusing if document.domain was set.

Comment 3

[Anne (:annevk)]

FWIW, happy to patch the HTML Standard once we're ready to discuss it in public.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1552541. Consider document.domain when deciding on inner window reuse. r=bholley

Comment 5

[Boris Zbarsky [:bzbarsky]]

I did not include a test in the patch, because I wanted the patch to reveal as little information as possible, for the moment... I figure we'll add a wpt when we change the HTML spec, and can add the eval version of the testcase once we open up this bug.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 9066170 [details]
Bug 1552541. Consider document.domain when deciding on inner window reuse. r=bholley

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Unclear. For someone who knows browsers, getting into the situation of inner window reuse when document.domain is involved is not hard. Going from there to an actual exploit would take some thinking, but probably not that long: it took me and bholley (mostly bholley) just a few minutes of talking about it to come up with something.

This is somewhat mitigated by the fact that only sites that use document.domain are affected, and only via allowing parts of their subdomain to attack other parts, so the severity of the attack is not as high as it could be.

• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which older supported branches are affected by this flaw?: All of them.
• If not all supported branches, which bug introduced the flaw?: Bug 956382
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?: This patch applies as-is to ESR 60 and presumably everything after that.
• How likely is this patch to cause regressions; how much testing does it need?: I expect regression risk to be low, since this matches the behavior of other browsers and our behavior from a long time ago, but unfortunately no guarantees, esp. if sites are doing Firefox-specific stuff.

Comment 7

[Boris Zbarsky [:bzbarsky]]

sec-high, I think, since this is XSS, albeit only under some circumstances.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 9066170 [details]
Bug 1552541. Consider document.domain when deciding on inner window reuse. r=bholley

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration:
• User impact if declined: Possible XSS on some sites that use document.domain, but only from other domains under the same eTLD+1
• Fix Landed on Version: 69
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Aligns behavior with other browsers. Should be reasonably safe.
• String or UUID changes made by this patch: None.

Beta/Release Uplift Approval Request

• User impact if declined: Possible XSS on some sites that use document.domain, but only from other domains under the same eTLD+1
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See comment 0. Unfortunately, this requires hosting a testcase somewhere with multiple subdomains, which is not trivial.... I unfortunately do not have a place to suggest for that.

I could write a testcase and post it on web.mit.edu if that would help.

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Aligns behavior with other browsers. Should be reasonably safe.
• String changes made/needed: None.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No

For what it's worth, bholley thinks the patch itself does.

Comment 10

[Al Billings [:abillings - ex-MoCo]]

sec-approval for checkin on June 4, two weeks into the new cycle. I don't think we want this in quite yet.

Comment 11

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/autoland/rev/79833c5d37bf7a68d693efe1c3d2614c559f68cb

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/79833c5d37bf

Comment 13

[Julien Cristau [:jcristau]]

Comment on attachment 9066170 [details]
Bug 1552541. Consider document.domain when deciding on inner window reuse. r=bholley

sec fix, approved for 68.0b8

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/9b3c45bd2e77

Comment 15

[Anne (:annevk)]

I created https://github.com/whatwg/html/security/advisories/GHSA-3wq5-39m2-7888 to track this standards-wise. Let me know if you need access (I'll need your GitHub ID).

Comment 16

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

Boris, can you please post the test case on web.mit.edu so we can manually verify this issue?
Thank you!

Comment 17

[Anne (:annevk)]

Cristian, created https://cyan-cork-1.glitch.me/ for you. (I can recommend signing up for Glitch for things like this, it's good stuff.)

In Nightly I see undefined and while the eval call does not throw, it doesn't affect anything currently open. (I don't quite recall how that's defined. Perhaps it does throw but it's not a window the developer tools show?)

Comment 18

[Boris Zbarsky [:bzbarsky]]

There's a version of that testcase temporarily at http://web.mit.edu/bzbarsky/www/test12.html too. It's not likely to last more than a few days.

while the eval call does not throw, it doesn't affect anything currently open

It evals in the scope of the about:blank thing, which got unloaded. It does modify the DOM of that unloaded thing; no throwing involved.

Comment 19

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

Thank you Anne and Boris for your help!
I reproduced this issue using Fx 68.0a1 (2019-05-17), on Windows 10 x64.
I can confirm this issue is fixes, I verified using Fx 69.0a1(2019-06-10) and Fx 68.0b9.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9066170 [details]
Bug 1552541. Consider document.domain when deciding on inner window reuse. r=bholley

Fixes a possible sec-high XSS bug. Verified on Nightly and Beta. Approved for 60.8esr.

Comment 21

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-esr60/rev/c1a5c7102567

Comment 22

[Bogdan Maris, Desktop QA]

(In reply to Cristian Comorasu, QA [:ccomorasu], Release Desktop QA from comment #19)

Thank you Anne and Boris for your help!
I reproduced this issue using Fx 68.0a1 (2019-05-17), on Windows 10 x64.
I can confirm this issue is fixes, I verified using Fx 69.0a1(2019-06-10) and Fx 68.0b9.

Also verified as fixed using latest esr from taskcluster https://tools.taskcluster.net/index/gecko.v2.mozilla-esr60.pushdate.2019.06.20.20190620183641.firefox across platforms (Windows 10 x64, Ubuntu 18.04 x64 and macOS 10.13).

Comment 23

[Al Billings [:abillings - ex-MoCo]]

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #1)

Credit for finding this should go to [e-mail], by the way.

Do we have a name for this as I'm doing an advisory or do I need to send an email blindly and ask who they are? :-)

Comment 24

[Boris Zbarsky [:bzbarsky]]

I don't have a name, sorry. The original discovery was in bug 1551272, where they found a real-life testcase which was caused by this problem. They are the reporter of bug 1551272.