Bug 803870 (CVE-2013-0793)

XSS using timed document navigations

VERIFIED FIXED in Firefox 20

Status

()

Core
Document Navigation
VERIFIED FIXED
5 years ago
3 years ago

People

(Reporter: Mariusz Mlynski, Assigned: bholley)

Tracking

({sec-high})

16 Branch
mozilla22
sec-high
Points:
---
Bug Flags:
sec-bounty +
in-testsuite +

Firefox Tracking Flags

(blocking-b2g:tef+, firefox18+ wontfix, firefox19+ wontfix, firefox20+ verified, firefox21+ verified, firefox22+ verified, firefox-esr10 wontfix, firefox-esr1720+ verified, b2g1820+ fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 fixed)

Details

(Whiteboard: [adv-main20+][adv-esr1705+])

Attachments

(4 attachments)

(Reporter)

Description

5 years ago
It is possible to load an arbitrary website with the document's baseURI pointing to a malicious server, which allows script injection or data theft when the page accesses a relative path.

The exploit performs several navigations to achieve a stable result. It seems to me that the crucial part is the following sequence:

opener.history.go(0);
opener.history.forward();
opener.location.hash='a';

In certain circumstances, the late hash navigation appears to confuse the browser to adopt the same BFCache and/or share a history entry between the 2 adjacent, cross-origin pages.
(Reporter)

Comment 1

5 years ago
Created attachment 673604 [details]
Proof of concept

It opens https://bug757376.bugzilla.mozilla.org/attachment.cgi?id=626233 which loads script.js from a relative path after a few seconds. The script will be fetched from whatever location you upload this testcase to, so don't run it from the file system.

Tested on the current release build (16.0.1) and the latest Nightly (19.0a1, 2012-10-20).
(Reporter)

Updated

5 years ago
Attachment #673604 - Attachment mime type: application/octet-stream → application/java-archive
(Reporter)

Comment 2

5 years ago
Comment on attachment 673604 [details]
Proof of concept

Ugh, this doesn't appear to work from jar:, presumably due to identical URLs getting a different "t" parameter on consecutive loads -- maybe it spoils the caching somehow. Just assume to test it from another web server.
Attachment #673604 - Attachment mime type: application/java-archive → application/zip
Sounds awfully like bug 775009 and friends...
Oh jeez.
Bug 801576 should make this a benign bug, rather than a security bug.
(In reply to Boris Zbarsky (:bz) from comment #3)
> Sounds awfully like bug 775009 and friends...

Same reporter, guess the fix there wasn't sufficient.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: sec-high

Updated

5 years ago
Flags: sec-bounty?
Attachment #673604 - Attachment mime type: application/zip → application/java-archive
status-firefox18: --- → affected
status-firefox19: --- → affected
status-firefox20: --- → affected
status-firefox-esr17: --- → affected
tracking-firefox18: --- → +
tracking-firefox19: --- → +
tracking-firefox20: --- → +
tracking-firefox-esr17: --- → ?
Assignee: nobody → justin.lebar+bug
(In reply to Justin Lebar [:jlebar] from comment #5)
> Bug 801576 should make this a benign bug, rather than a security bug.

That bug is a web compat change, can we take it on ESR-17?
Assignee: justin.lebar+bug → bobbyholley+bmo

Comment 9

5 years ago
Also, is ESR10 affected?
(In reply to Daniel Veditz [:dveditz] from comment #8)
> (In reply to Justin Lebar [:jlebar] from comment #5)
> > Bug 801576 should make this a benign bug, rather than a security bug.
> 
> That bug is a web compat change, can we take it on ESR-17?

Possibly, but only once that bug bubbles all the way out to the release channel and we can confirm that there are no major web compat regressions. Otherwise, I don't think it's appropriate.

(In reply to Alex Keybl [:akeybl] from comment #9)
> Also, is ESR10 affected?

AFAICT yes.

As for this bug itself, I'm not the appropriate assignee.
Assignee: bobbyholley+bmo → justin.lebar+bug
Until B2G calms down, I'm really not going to be able to give this bug the attention it deserves.

I'm also hesitant to spend a lot of time on this bug because we've already fixed three previous incarnations of it.  It seems likely that if we fix this bug, there will be yet another hole.

Moreover a direct fix here will be risky for branches, because it's hard to tell when we're breaking an edgecase in docshell.  We've regressed peoples' banking sites at least once trying to fix a similar bug.

Bug 801576 seems like a reasonable fix to me.  If we cannot wait for that to percolate to branches, I'm likely the wrong person to write a fix for this bug for branches.
status-firefox-esr10: --- → affected
Bobby, thoughts on last paragraph of comment 11?
(In reply to Boris Zbarsky (:bz) from comment #12)
> Bobby, thoughts on last paragraph of comment 11?

As noted in comment 10, I don't think its appropriate to uplift bug 801576.

As for this bug, I don't understand what's going on. But it seems to be continuing drama from an issue that Justin seems to understand, so I'll defer to him on whether revoking cross-origin History access is enough here. Or should I spend some time and dig in here?
Revoking cross-origin history access is sufficient to fix this bug, yes.
> Bug 801576 seems like a reasonable fix to me.  If we cannot wait for that to percolate to 
> branches, I'm likely the wrong person to write a fix for this bug for branches.

To be clear, I wasn't arguing that we should or shouldn't take bug 801576 on branches, just that it's the path of least resistance for fixing this bug and the myriad ones like it (or at least, turning them into regular bugs instead of security bugs).
(In reply to Justin Lebar [:jlebar] from comment #11)
> Bug 801576 seems like a reasonable fix to me.

If that's a fix for this bug then this should be fixed in 19 and 20 already. Can we verify that?
Flags: needinfo?(mwobensmith)
Since we're worried about web-compat I can't see us taking this on ESR10. After we've gotten some beta exposure for the fix in bug 801576 we can then uplift to ESR-17 in the Fx19 timeframe if we don't break too much on the web.
status-firefox-esr10: affected → wontfix

Updated

5 years ago
status-firefox18: affected → wontfix
tracking-firefox-esr17: ? → 19+

Comment 18

5 years ago
(In reply to Daniel Veditz [:dveditz] from comment #16)
> (In reply to Justin Lebar [:jlebar] from comment #11)
> > Bug 801576 seems like a reasonable fix to me.
> 
> If that's a fix for this bug then this should be fixed in 19 and 20 already.
> Can we verify that?

The fix in bug 801576 does not entirely revoke cross-origin history access.  Once a script gets a history object from a same-origin window, the script can call back(), forward(), and go() methods even when that window is no longer same-origin.

I'll attach a modified version of the reporter's testcase.

Comment 19

5 years ago
Created attachment 692846 [details]
modified version of navigate.htm

I can reproduce the reporter's testcase on fx19 and fx20 by replacing the original file with this file.
Bobby, is that intentional behavior?
(In reply to Justin Lebar [:jlebar] from comment #20)
> Bobby, is that intentional behavior?
Which behavior, exactly? History operating on the outer window (rather than the inner)? What would you prefer it to do? Throw if its window isn't the current inner?
> What would you prefer it to do? Throw if its window isn't the current inner?

Throw if the caller's principal doesn't match the inner window's principal?  That is, throw iff the caller's reading |openee.location| would throw?
(In reply to Justin Lebar [:jlebar] from comment #22)
> > What would you prefer it to do? Throw if its window isn't the current inner?
> 
> Throw if the caller's principal doesn't match the inner window's principal? 
> That is, throw iff the caller's reading |openee.location| would throw?

Ok, that would involves a couple of simple C++ security checks in nsHistory.cpp (in back, forward, and go, I'd imagine?). Just adding a static method that compares GetWindow to do_QueryInterface<nsPIDOMWindow>(GetDocShell())->GetCurrentInnerWindow() should be sufficient.

Note that this approach is only landable on branches that have bug 801576. That is to say, it only gets us back to where we thought we were before comment 18.
As moz_bug_r_a4 states above, this issue can be reproduced in 20 and 19 with a modified test case.
Flags: needinfo?(mwobensmith)
Attachment #692846 - Attachment mime type: text/plain → text/html
Flags: sec-bounty? → sec-bounty+
We're only a couple of weeks away from the release of FF19. Are we still hoping to fix in that timeframe?
tracking-firefox-esr17: 19+ → ---
I would have to drop some high-priority B2G work in order to get a fix for this in docshell.  But moreover, every one of the past three fixes for this sort of bug I've come up with have been worked around, so it seems sisyphean to continue like that when we have an alternative fix.

IOW, I don't think I'm the right assignee for this bug.
Assignee: justin.lebar+bug → nobody
(In reply to Justin Lebar [:jlebar] from comment #26)
> I would have to drop some high-priority B2G work in order to get a fix for
> this in docshell.  But moreover, every one of the past three fixes for this
> sort of bug I've come up with have been worked around, so it seems sisyphean
> to continue like that when we have an alternative fix.

What is the alternate fix? We have bug 801576, which will be released in 19, but per comment 23 that still won't fix things unless we do a little bit of extra work, right?
> but per comment 23 that still won't fix things unless we do a little bit of extra work, right?

Right.
Any good alternative assignees for the extra work?
bobby: are you the right one to do what you suggested in comment 23?
Assignee: nobody → bobbyholley+bmo
status-b2g18: --- → affected
Ok, I'll write a patch.
Batch edit: Bugs marked status-b2g18: affected after 2/13 branching of v1.0.1 are now also status-b2g18-v1.0.1: affected
status-b2g18-v1.0.1: --- → affected
Created attachment 715257 [details] [diff] [review]
Compare subject with the outer window when invoking methods. v1
Attachment #715257 - Flags: review?(bzbarsky)
Created attachment 715258 [details] [diff] [review]
Tests. v1
Attachment #715258 - Flags: review?(bzbarsky)
Attachment #715257 - Attachment description: Compare History with the outer window when invoking methods. v1 → Compare subject with the outer window when invoking methods. v1
https://tbpl.mozilla.org/?tree=Try&rev=79b1430a1b2a
Comment on attachment 715257 [details] [diff] [review]
Compare subject with the outer window when invoking methods. v1

r=me
Attachment #715257 - Flags: review?(bzbarsky) → review+
Oh, and this probably needs to have a spec issue raised.
Comment on attachment 715258 [details] [diff] [review]
Tests. v1

r=me
Attachment #715258 - Flags: review?(bzbarsky) → review+
Comment on attachment 715257 [details] [diff] [review]
Compare subject with the outer window when invoking methods. v1

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

It's clear that we're preventing access to History if the inner is not the current active Window. However, these methods were accessible by any frame, even cross-origin, until very recently (bug 801576). This patch doesn't do much to reveal the issue in comment 0.

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

See above.

Which older supported branches are affected by this flaw?

All of them, I think.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

No, but it should be straightforward. Note that backporting this to esr17 would also involve backporting bug 801576, which is a behavior change. But given that we haven't heard any fallout, that might be acceptable? Probably worth asking akeybl.

How likely is this patch to cause regressions; how much testing does it need?

Not risky.
Attachment #715257 - Flags: sec-approval?
Attachment #715257 - Flags: sec-approval? → sec-approval+
status-firefox21: --- → affected
status-firefox22: --- → affected
tracking-firefox21: --- → +
tracking-firefox22: --- → +
https://hg.mozilla.org/integration/mozilla-inbound/rev/bd115faef0ee
Comment on attachment 715257 [details] [diff] [review]
Compare subject with the outer window when invoking methods. v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): longstanding
User impact if declined: security bugs
Testing completed (on m-c, etc.): Just pushed to m-i
Risk to taking this patch (and alternatives if risky): not risky.
String or UUID changes made by this patch: none

Note that landing this on esr17 requires backporting bug 801576, which is another discussion. Flagging just m-a and m-b for now.
Attachment #715257 - Flags: approval-mozilla-beta?
Attachment #715257 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/bd115faef0ee
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-firefox22: affected → fixed
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
status-firefox19: affected → wontfix
tracking-b2g18: --- → 20+
tracking-firefox-esr17: --- → 20+
(In reply to Bobby Holley (:bholley) from comment #41)
> Note that landing this on esr17 requires backporting bug 801576, which is
> another discussion.

I'd still like to take it. Not worried about the compat on the public web since mainline Firefox will force updates there, but slight risk of compat issues for internal apps.
Depends on: 801576
Thanks a lot for fixing this bug, Bobby.
(In reply to Daniel Veditz [:dveditz] from comment #43)
> (In reply to Bobby Holley (:bholley) from comment #41)
> > Note that landing this on esr17 requires backporting bug 801576, which is
> > another discussion.
> 
> I'd still like to take it. Not worried about the compat on the public web
> since mainline Firefox will force updates there, but slight risk of compat
> issues for internal apps.

Ok, I'll flag that bug for uplift.

(In reply to Justin Lebar [:jlebar] from comment #44)
> Thanks a lot for fixing this bug, Bobby.

No problem. :-)
Comment on attachment 715257 [details] [diff] [review]
Compare subject with the outer window when invoking methods. v1

Approving for uplift to branches, let the discussion of ESR continue...
Attachment #715257 - Flags: approval-mozilla-beta?
Attachment #715257 - Flags: approval-mozilla-beta+
Attachment #715257 - Flags: approval-mozilla-aurora?
Attachment #715257 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/193316bdc446
https://hg.mozilla.org/releases/mozilla-beta/rev/98579b64434a
status-firefox20: affected → fixed
status-firefox21: affected → fixed

Comment 48

4 years ago
Verified as fixed on:
Mozilla/5.0 (X11; Linux i686; rv:20.0) Gecko/20100101 Firefox/20.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20100101 Firefox/20.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:20.0) Gecko/20100101 Firefox/20.0
status-firefox20: fixed → verified
Comment on attachment 715257 [details] [diff] [review]
Compare subject with the outer window when invoking methods. v1

Bug 801576 has approval-esr17 now, so flagging this for uplift as well. Once this is approved, I'll push them both together.
Attachment #715257 - Flags: approval-mozilla-esr17?
Whiteboard: [leave uplift for bholley]
Ryan successfully uplifted bug 801576, so the uplift here should be relatively straightforward. If you want to take a crack at it Ryan, feel free. :-)
Whiteboard: [leave uplift for bholley]

Updated

4 years ago
Attachment #715257 - Flags: approval-mozilla-esr17? → approval-mozilla-esr17+
https://hg.mozilla.org/releases/mozilla-esr17/rev/f1cb8d49c4d7

I assume you'll be landing the test on the branches once Fx20 is released? Also, still wondering about whether this needs to land on the b2g18 branch(es) or not.
status-b2g18-v1.0.0: --- → wontfix
status-firefox-esr17: affected → fixed
Comment on attachment 715257 [details] [diff] [review]
Compare subject with the outer window when invoking methods. v1

(In reply to Ryan VanderMeulen [:RyanVM] from comment #51)
> https://hg.mozilla.org/releases/mozilla-esr17/rev/f1cb8d49c4d7
> 
> I assume you'll be landing the test on the branches once Fx20 is released?

Yes, I'm watching my in-testsuite? bugs and will batch land the tests at some point.

> Also, still wondering about whether this needs to land on the b2g18
> branch(es) or not.

Let's find out. Nominating.
Attachment #715257 - Flags: approval-mozilla-b2g18?

Updated

4 years ago
status-b2g18-v1.0.1: affected → wontfix

Updated

4 years ago
Attachment #715257 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
https://hg.mozilla.org/releases/mozilla-b2g18/rev/49ed11e7cb79
status-b2g18: affected → fixed
Flagging for verification in Firefox 21, 22, and esr17.
Keywords: verifyme
(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #54)
> Flagging for verification in Firefox 21, 22, and esr17.

Kamil, can you please help us out with verifying this bug?
Verified on m-c 22, 2013-03-12.
status-firefox22: fixed → verified
Verified on Aurora 21, 2013-03-12.
status-firefox21: fixed → verified
Verified on ESR 17.0.3, 2013-03-08.
Status: RESOLVED → VERIFIED
status-firefox-esr17: fixed → verified
Keywords: verifyme
Whiteboard: [adv-main20+][adv-esr1705+]
Alias: CVE-2013-0793
Since Firefox 20 is now released, v1.0.1 branches are still open, and this is considered low risk, we can uplift to v1.0.1. Marking as tef+ and flipping status-b2g18-v1.0.1 to affected.
blocking-b2g: --- → tef+
status-b2g18-v1.0.1: wontfix → affected
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/fdc04dffa97b

I also pushed bug 801576 with a=akeybl over IRC.
status-b2g18-v1.0.1: affected → fixed
I'm going to fold the tests for this into my tests for bug 940783.
Flags: in-testsuite? → in-testsuite+
Group: core-security
You need to log in before you can comment on or make changes to this bug.