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.
Created attachment 366276 [details] 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.
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.
(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.
(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).
> 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.
Created attachment 366467 [details] [diff] [review] 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.
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.
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. :(
The "Proposed fix" patch applies directly to 1.9.0 and fixes the bug there too.
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?
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?
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.
OK. I'll do the usual "wait till we ship" thing, I guess. I hate that. :(
Pushed http://hg.mozilla.org/releases/mozilla-1.9.1/rev/9a1584533724 without the test.
Comment on attachment 366467 [details] [diff] [review] Proposed fix Need this on 1.9.0 as well.
Comment on attachment 366467 [details] [diff] [review] Proposed fix Approved for 188.8.131.52, a=dveditz for release-drivers
Checked into CVS, still without test.
Verified for 184.108.40.206 on XP with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:220.127.116.11pre) 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.