Closed
Bug 799952
(CVE-2012-4192)
Opened 12 years ago
Closed 12 years ago
Cross domain access to the location object
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
VERIFIED
FIXED
mozilla18
Tracking | Status | |
---|---|---|
firefox15 | --- | unaffected |
firefox16 | + | verified |
firefox17 | + | verified |
firefox18 | --- | unaffected |
firefox19 | --- | unaffected |
firefox-esr10 | --- | unaffected |
People
(Reporter: ehsan.akhgari, Assigned: ejpbruel)
References
()
Details
(5 keywords, Whiteboard: fixed by bug 720619)
Attachments
(5 files, 2 obsolete files)
http://www.thespanner.co.uk/2012/10/10/firefox-knows-what-your-friends-did-last-summer/
function poc() {
var win = window.open('https://twitter.com/lists/', 'newWin', 'width=200,height=200');
setTimeout(function(){
alert('Hello '+/^https:\/\/twitter.com\/([^/]+)/.exec(win.location)[1])
}, 5000);
}
I can't reproduce in 15 or trunk, but 16 is affected.
Reporter | ||
Updated•12 years ago
|
Keywords: regression,
regressionwindow-wanted
Group: core-security
tracking-firefox16:
--- → ?
This also affects 17 (at least 10/8's Aurora) but not trunk.
status-firefox19:
--- → unaffected
tracking-firefox17:
--- → ?
Updated•12 years ago
|
status-firefox16:
--- → affected
status-firefox17:
--- → affected
Comment 2•12 years ago
|
||
See also bug 730431 and bug 720619.
This doesn't involve the web console. Turn off popups and visit http://people.mozilla.org/~khuey/test_location.html
It's also not the same as Bug 720619 because it doesn't happen in the copy of ESR that I have, but the fix for that has not been backported to ESR yet.
Comment 4•12 years ago
|
||
Note that the comments on the linked blog also say that both
alert(document.getElementById(‘x’).contentWindow.location)
and
alert(win.location)
work.
Comment 5•12 years ago
|
||
I'll attach a minimal testcase in a sec that does not involve popups.
This regressed on trunk in the range http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=bf8f2961d0cc&tochange=4a8e0d5fc954 so presumably from bug 754202. It then got fixed in bug 720619, I assume.
Blocks: 754202
Comment 6•12 years ago
|
||
Updated•12 years ago
|
status-firefox15:
--- → unaffected
status-firefox18:
--- → unaffected
We should probably pull 16 off the wire.
Comment 8•12 years ago
|
||
This actually hits a different exception in 15 than the "stupid testcase" does, for what it's worth.
Comment 9•12 years ago
|
||
One thing I can't understand is how we could possibly not have had a test for this.
Reporter | ||
Updated•12 years ago
|
OS: Mac OS X → All
Hardware: x86 → All
Version: Trunk → 16 Branch
Comment 10•12 years ago
|
||
And specifically, this regressed with http://hg.mozilla.org/mozilla-central/rev/6f70fe755b10 which is what actually switched from pulling principals off the stackframe to pulling them off the compartment...
Updated•12 years ago
|
Keywords: regressionwindow-wanted → testcase
Updated•12 years ago
|
Comment 11•12 years ago
|
||
I'm landing this over in bug 720619. Marking the dep.
Depends on: CVE-2012-4193
Updated•12 years ago
|
Updated•12 years ago
|
Updated•12 years ago
|
Whiteboard: fixed by bug 720619
Comment 13•12 years ago
|
||
I've tested to make sure that ESR10 is unaffected here.
status-firefox-esr10:
--- → unaffected
Comment 14•12 years ago
|
||
I'm working with Boris and will be able to check in test cases very soon.
Comment 15•12 years ago
|
||
Matt Wobensmith and I have confirmed the testcases in this bug and bug 720619 reproduce in 17b1#1 and do not reproduce in 17b1#2. We only have candidates for Linux and Mac so far so we can't call this verified yet, but at least Linux & Mac are taken care of.
Comment 16•12 years ago
|
||
I've tested this on Linux and Mac on builds prior to the fix (where I can see the problem), and with the Fx16.0.1 candidates where I do not see the problem. I'll check Windows when the builds are out.
Comment 17•12 years ago
|
||
Verified Fixed against Firefox for Android (mozilla-release, 16.0.1)
Test-case: public PoC
Result: No alert; console error — "Permission denied to access property 'toString'"
Test-case: first attached test-case
Result: Alert — NS_ERROR_XPC_BAD_CONVERT on argument 0
Test-case: second attached test-case
Result: Alert — Error: "Permission denied to access property 'toString'"
Comment 18•12 years ago
|
||
I've verified using the test cases here and in bug 720619 on Windows XP and Fx16.0.1 candidates.
Comment 19•12 years ago
|
||
As per discussion with Boris, this test fleshes out the issue a bit more and adds a few more cases not covered by simple access to the location object. Also includes some positive test cases in case we broke same-domain location object access.
All tests pass in FF15. Some break in 17.0b1 build 1 but then are all fixed in 17.0b1 build 2.
Also worth noting: try running this in Safari and Chrome. Just sayin'.
Comment 20•12 years ago
|
||
(In reply to Matt Wobensmith from comment #19)
> Created attachment 670212 [details]
> Positive/negative test cases for accessing cross- and same-domain location objects
> [...]
'All tests pass' on Firefox for Android (mozilla-release, 16.0.1)
(In reply to Matt Wobensmith from comment #19)
> Created attachment 670212 [details]
> Positive/negative test cases for accessing cross- and same-domain location
> objects
>
> As per discussion with Boris, this test fleshes out the issue a bit more and
> adds a few more cases not covered by simple access to the location object.
> Also includes some positive test cases in case we broke same-domain location
> object access.
>
> All tests pass in FF15. Some break in 17.0b1 build 1 but then are all fixed
> in 17.0b1 build 2.
>
> Also worth noting: try running this in Safari and Chrome. Just sayin'.
On Chrome and Opera I see "Fail: X-domain access to iframe contentWindow.location object via Object.prototype.toString.call should be disallowed"
Using Object.prototype.toString.call just returns "[Object Location]" though, so I don't think there's a security issue here in Chrome (or Opera).
Comment 22•12 years ago
|
||
* More verbose output (to show that Chrome's behavior is probably okay)
* Use https://blog.mozilla.com/ instead of http://web.mit.edu/ so we don't hit mixed content blocking when loading the testcase from Bugzilla
* setTimeout instead of setInterval
Updated•12 years ago
|
Alias: CVE-2012-4192
Comment 23•12 years ago
|
||
Since this is public, are we intending to do a chemspill to fix this one? its sg-critical from what i see.
Updated•12 years ago
|
Comment 24•12 years ago
|
||
Verified fixed with all attached testcases in Firefox 17.0b1#2 on Ubuntu 12.04, Mac OSX 10.7, and Windows 7. Any reason to keep this bug NEW?
Keywords: verifyme
Updated•12 years ago
|
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 26•12 years ago
|
||
(In reply to Huzaifa Sidhpurwala from comment #23)
> Since this is public, are we intending to do a chemspill to fix this one?
> its sg-critical from what i see.
It is sg-high (there's no remote code execution). We are indeed chemspilling.
Updated•12 years ago
|
Status: RESOLVED → VERIFIED
Comment 27•12 years ago
|
||
Thanks Kyle and Jesse. Indeed, my test had the false positive for the case where [Object Location] was returned. Thanks for tweaking it.
Comment 28•12 years ago
|
||
These testcases all report PASS in the 10.0.9esr candidate builds.
Comment 29•12 years ago
|
||
Can this bug report be made public?
Updated•12 years ago
|
Group: core-security
Comment 30•12 years ago
|
||
Can we clear the in-testsuite flag given that we got a testcase with the fix on bug 720619?
Comment 31•12 years ago
|
||
The testcase in bug 720619 does not test the web-visible codepaths. The testcase in this bug does. What needs to happen is for the testcase in this bug to be added to our automated tests.
Updated•12 years ago
|
Attachment #671644 -
Attachment description: Bug Bounty Awarded $3000 → Bug Bounty Awarded $3000 [offer declined]
Comment 32•12 years ago
|
||
Attachment #673487 -
Flags: review?(bzbarsky)
Comment 33•12 years ago
|
||
Comment on attachment 673487 [details] [diff] [review]
testcase to bug 799952 - x-domain access to location objects
>+++ b/dom/tests/mochitest/bugs/Makefile.in
>+ test_bug799952.html \
child_bug799952.html \
This is broken. You have a CR in there after the backslash, not a newline. Please fix that.
>+++ b/dom/tests/mochitest/bugs/child_bug799952.html
>@@ -0,0 +1,1 @@
>+#799952
How about:
<!DOCTYPE html>
at least? Or just name the file child_bug799952.txt, perhaps?
>+++ b/dom/tests/mochitest/bugs/test_bug799952.html
>+setTimeout (start, 2000);
So as a general rule, use of setTimeout in a test indicates a bug.
In this case, nothing actually ensures that after two seconds the frame has even been parsed, much less finished loading. It would be better to do:
addLoadEvent(start);
which will make sure start() runs after onload.
>+// the following are error cases. If they do not throw, they are failures.
Probably good to indent everything inside the function by two spaces.
Also, the try/catch indentation is slightly odd. Normally I think it would be:
try {
something;
} catch (e) {
whatever;
}
>+ foo = String (/(.*)/.exec(window[0].location)[1]);
Perhaps replace window[0] with $("win").contentWindow like you have for the other tests here?
>+is ( result, "Error: Permission denied to access property \'toString\'", "Access to xdomain location via regexp must throw exception" )
Please nix the spaces around the '(' and before the ')'.
>+<iframe id="win" name="win" src="https://example.com:443/tests/dom/tests/mochitest/bugs/child_bug799952.html"></iframe>
You don't need a "name" attribute there.
>+<iframe id="sameDomainContent" name="sameDomainContent" src="../child_bug799952.html"></iframe>
Likewise. And also, why the "../" bit?
r=me with the above addressed. Thank you for writing these up!
Attachment #673487 -
Flags: review?(bzbarsky) → review+
Comment 34•12 years ago
|
||
Incorporated Boris' changes.
Attachment #673487 -
Attachment is obsolete: true
Attachment #674414 -
Flags: review?(bzbarsky)
Comment 35•12 years ago
|
||
Comment on attachment 674414 [details] [diff] [review]
Updated test case for bug #799952
> +<iframe id="sameDomainContent" src="/tests/dom/tests/mochitest/bugs/child_bug799952.html"></iframe>
Can't this just be:
<iframe d="sameDomainContent" src="child_bug799952.html"></iframe>
?
All the is() calls should have the third argument indented even with the first argument, not with the is() itself.
r=me with those nits picked.
Attachment #674414 -
Flags: review?(bzbarsky) → review+
Comment 36•12 years ago
|
||
Problem with referencing simply "child_bug799952.html" is that the Mochitest test suite - for whatever reason - tries to resolve that as "test_bug799952.html/child_bug799952.html" which returns a 404.
Previously I used a workaround of "../" but you mentioned that wasn't kosher.
Any clues about why relative URLs are resolved like this? I'm pretty baffled. The above solution works and assures us that we have a same-domain URL, but it does feel like I'm missing something.
Comment 37•12 years ago
|
||
> is that the Mochitest test suite - for whatever reason - tries to resolve that as
> "test_bug799952.html/child_bug799952.html"
That's.... quite unexpected. How exactly are you running the test?
Comment 38•12 years ago
|
||
Fixed URL issue. Looks like I was adding an extra slash to the file name when I ran the test locally, causing the 404 badness. Mea culpa and thanks for having patience.
Attachment #674414 -
Attachment is obsolete: true
Attachment #674785 -
Flags: review+
Updated•12 years ago
|
Keywords: checkin-needed
Comment 39•12 years ago
|
||
Pushed to Try:
https://tbpl.mozilla.org/?tree=Try&rev=aa7a833ce345
Comment 40•12 years ago
|
||
Assignee: nobody → ejpbruel
Flags: in-testsuite? → in-testsuite+
Keywords: checkin-needed
Target Milestone: --- → mozilla18
Comment 41•12 years ago
|
||
Sorry, backed out in http://hg.mozilla.org/integration/mozilla-inbound/rev/305471f75b59 - it's only run a few times, and it's already failed in https://tbpl.mozilla.org/php/getParsedLog.php?id=16441596&tree=Mozilla-Inbound and https://tbpl.mozilla.org/php/getParsedLog.php?id=16441901&tree=Mozilla-Inbound and https://tbpl.mozilla.org/php/getParsedLog.php?id=16442191&tree=Mozilla-Inbound
Comment 42•12 years ago
|
||
(In reply to Phil Ringnalda (:philor) from comment #42)
> Sorry, backed out in
> http://hg.mozilla.org/integration/mozilla-inbound/rev/305471f75b59 - it's
> only run a few times, and it's already failed in
> https://tbpl.mozilla.org/php/getParsedLog.php?id=16441596&tree=Mozilla-
> Inbound and
> https://tbpl.mozilla.org/php/getParsedLog.php?id=16441901&tree=Mozilla-
> Inbound and
> https://tbpl.mozilla.org/php/getParsedLog.php?id=16442191&tree=Mozilla-
> Inbound
Resetting in-testsuite flag.
Flags: in-testsuite+ → in-testsuite?
Comment 43•12 years ago
|
||
That's .... really odd. Those failures look like what I would expect if the test were running before the cross-origin iframe has loaded, but we're explicitly running it off the load event! What the heck?
Comment 44•12 years ago
|
||
Maybe add a test that makes sure we can't read the DOM of the cross-origin iframe and push to try with that? See whether that test is also failing when the location part fails?
Alternately, perhaps log the actual location string when we get it, so we can see what page we _think_ we have loaded in there.
Updated•11 years ago
|
Flags: sec-bounty+
Updated•11 years ago
|
Attachment #671644 -
Attachment description: Bug Bounty Awarded $3000 [offer declined] → Bug Bounty Awarded $3000 [offer declined] [no response]
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
Updated•4 years ago
|
Flags: sec-bounty-hof-
You need to log in
before you can comment on or make changes to this bug.
Description
•