482206 - [FIX]It's possible to create a document whose URI does not match the document's principal

Status: VERIFIED FIXED

(Core :: Security, defect)

Description

[moz_bug_r_a4]

This seems to be a regression from bug 445004.  fx3.0.7 and fx3.1 are affected.
On trunk, this seems to have been fixed by bug 465806.

In nsHTMLDocument::OpenCommon(), we get the URI from the document returned by
GetDocumentFromContext(), but we get the principal by using
GetSubjectPrincipal().

An attacker can create a fake login page whose URI is a real login page's URI. 
And, if a password for the real login page was already saved in the Password
Manager, an attacker can steal the password without user interaction.

Comment 1

[moz_bug_r_a4]

Attachment: testcase

Steps to reproduce:
1. Load a testcase.
2. Wait a few second.

A fake login page whose URI is
https://addons.mozilla.org/en-US/firefox/users/login appears.

And, if you already saved a password for
https://addons.mozilla.org/en-US/firefox/users/login then the Password Manager
will automatically fill in the password.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Hmm.  So I'd be fine with backporting bug 465806 (and maybe adding a hack of some sort for system principals so as not to break extension compat).  jst, sicking, what do you think?

I guess the key to the testcase is that during execution of the function a() we actually change the principal and URI of the document whose context the function is running on.  So the function has the old principals (which are still same-origin with the parent page) while the URI of the subframe is now a different URI, as is its principal.  Is that correct?

I wish password manager didn't just rely on the URI; there are all sorts of ways the URI can be weird with document.write, especially once we implement the HTML5-specced behavior for the URI of document.write result pages.

Comment 3

[moz_bug_r_a4]

(In reply to comment #2)
> I guess the key to the testcase is that during execution of the function a() we
> actually change the principal and URI of the document whose context the
> function is running on.  So the function has the old principals (which are
> still same-origin with the parent page) while the URI of the subframe is now a
> different URI, as is its principal.  Is that correct?

Yes.

Comment 4

[Justin Dolske [:Dolske]]

(In reply to comment #2)

> I wish password manager didn't just rely on the URI; there are all sorts of
> ways the URI can be weird with document.write, especially once we implement the
> HTML5-specced behavior for the URI of document.write result pages.

What would it use instead?

If a page is able to bypass same-origin restrictions to reach into other content, it seems like that's the root problem that would lead to all kinds of other issues (say, stealing login cookies).

Comment 5

[Boris Zbarsky [:bzbarsky]]

> What would it use instead?

The principal.  That's what identifies what the page actually "is".  For example, for a data: URI it would have the actual originating URI.  That sort of thing.  It's probably a good idea to cross-check the two in the password manager case, really...

> If a page is able to bypass same-origin restrictions to reach into other
> content

All the script running in this case is script with the origin of the original outer document.  You did read the testcase, right?  There are no same-origin restrictions being bypassed.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed fix

This is basically the backport I described in comment 2, with a test that's based on the testcase in this bug but sufficiently different to hopefully make it unclear what the danger is...  I'd like to check in that same testcase to mozilla-central as well.  moz_bug_r, is that reasonable?

I've tested this on 1.9.1; waiting on my 1.9.0 builds.

Comment 7

[moz_bug_r_a4]

I don't think your testcase is sufficiently different.  If someone runs your
testcase in browser, he/she can realize that the URI in the location bar is the
cross-origin page's URI while content is "Something".  That is a real exploit
code, I guess.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Hmm.  I could obfuscate it a bit more by running the test in a subframe so that the URI is not obvious.  Would that help?

I can avoid pushing the test, but it seems a shame to have to do that.  :(

Comment 9

[Boris Zbarsky [:bzbarsky]]

The "Proposed fix" patch applies directly to 1.9.0 and fixes the bug there too.

Comment 10

[moz_bug_r_a4]

Once this bug is made public, attackers understand what they can do with this
bug.  So, even if the testcase in comment #1 is still private, they can easily
create a password stealing page based on your testcase.  Hmm, am I worrying
more than necessary?

Comment 11

[Boris Zbarsky [:bzbarsky]]

Once the bug is made public, I'm checking in the testcase no matter what, assuming I still remember to.  At that point we will have shipped fixes on all branches for the bug, right?

Comment 12

[moz_bug_r_a4]

Ah, checking in after shipping fixes is OK, indeed.

I'm not sure whether running the test in a subframe is helpful, if you check in
the testcase before shipping fixes.

Comment 13

[Boris Zbarsky [:bzbarsky]]

OK.  I'll do the usual "wait till we ship" thing, I guess.  I hate that.  :(

Comment 14

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/releases/mozilla-1.9.1/rev/9a1584533724 without the test.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 366467 [details] [diff] [review]
Proposed fix

Need this on 1.9.0 as well.

Comment 16

[Daniel Veditz [:dveditz]]

Comment on attachment 366467 [details] [diff] [review]
Proposed fix

Approved for 1.9.0.8, a=dveditz for release-drivers

Comment 17

[Boris Zbarsky [:bzbarsky]]

Checked into CVS, still without test.

Comment 18

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

Verified for 1.9.0.8 on XP with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.8pre) Gecko/2009031905 GranParadiso/3.0.8pre (.NET CLR 3.5.30729). 

Verified for 1.9.1 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090319 Shiretoko/3.5b4pre.