768101 - (CVE-2012-3993) XrayWrapper pollution via unsafe COW

Status: VERIFIED FIXED

(Core :: Security, defect)

Description

[Mariusz Mlynski]

Attachment: Proof of concept

When InstallTrigger fails, it throws an error wrapped in a Chrome Object Wrapper. It doesn't specify |__exposedProps__|, so it's unsafe by default. An attacker may append custom-tailored |__exposedProps__| at the bottom of the error's prototype chain (in Object.prototype), thus exposing privileged functions of Object or Function. Function seems to be guarded well (it throws "Not allowed to access chrome eval or Function from content"), but Object has things like defineProperty or __defineGetter__, and they can be exploited to pollute the XrayWrapper of the content window.

Comment 2

[Bobby Holley (:bholley)]

Sigh, it sure would have been nice if people would have let me land bug 553102 when I tried to back in May.

Anyway, that's the real fix here - everything else is just whack-a-mole. But it might be a game of whack-a-mole that we have to play.

Here's where the error is being thrown:
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/content/extensions-content.js#68

I think the sanest thing to do here in the interim is to special-case exception objects and make them default to deny.

Comment 3

[Bobby Holley (:bholley)]

(In reply to Bobby Holley (:bholley) from comment #2)
> I think the sanest thing to do here in the interim is to special-case
> exception objects and make them default to deny.

Oh right - but this is actually pulling the stuff off Object.prototype. Maybe we'll have to special-case that as well.

Comment 4

[Bobby Holley (:bholley)]

Hm, I guess this might as well be time to fix bug 760109.

Rather than trying to plug holes all the way up the prototype chain, I think it makes more sense for COW wrapping to detect if the proto of the wrapped object is a standard class, and if so, use the corresponding proto from the wrapper's compartment rather than the wrapped proto from the wrappee's compartment. This plugs the hole in the bug here, and also paves the way for a possible solution over in bug 760109: when the COW access check fails, we can just punt to our prototype and do the lookup there.

The easiest way to do this seems to be to add another wrapping callback to allow a custom prototype. 

One thing we would need would be a way to detect whether an object is a proto from a standard class. We can currently locate such protos from the global (JSProto_Object etc), but I don't see any way to go the other direction.

Luke, is there such a way? Or could we add one?

Comment 5

[Bobby Holley (:bholley)]

I'm almost finished with a set of patches for bug 760109 that fix this bug as a side effect. As such I'm going to use that bug as the primary vehicle, since it allows development to happen in the open, and since it gives us a plausible story for why we need to backport (making life more sensible for the addon developers who will be hit by the __exposedProps__ warning).

Comment 6

[Mariusz Mlynski]

Attachment: Local file theft demo using polluted Xrays (CRASHES)

I wanted to see how far I could go with overwritten Xrays without relying on add-ons or user interaction, and ended up with a local file theft demo. Here it is for the record, it forces a crash after a while and tries to get "C:\secret.txt" after the restart.

Comment 7

[chris hofmann]

any idea on a security rating for this bug?

Comment 8

[Bobby Holley (:bholley)]

Attachment: Imply empty __exposedProps__ for Error objects. v1

This is a small fix that _might_ be helpful in general, but doesn't fix the bug at hand. I'm leaning against landing it, since trunk will be fixed soon anyway (when we make __exposedProps__ mandatory), and would only be useful if chrome stuck custom properties on exceptions it threw (which is unlikely).

Comment 10

[Daniel Veditz [:dveditz]]

(In reply to Bobby Holley (:bholley) from comment #8)
> I'm leaning against landing it, since trunk will be fixed soon anyway
> (when we make __exposedProps__ mandatory),

A fix "soon" on trunk doesn't ship for 3-4 months, and doesn't help us on ESR at all. Any other options?

Comment 11

[Bobby Holley (:bholley)]

(In reply to Daniel Veditz [:dveditz] from comment #10)
> (In reply to Bobby Holley (:bholley) from comment #8)
> > I'm leaning against landing it, since trunk will be fixed soon anyway
> > (when we make __exposedProps__ mandatory),
> 
> A fix "soon" on trunk doesn't ship for 3-4 months, and doesn't help us on
> ESR at all. Any other options?

Perhaps I wasn't clear. I was referring to the patch in comment 8, which doesn't fix any known security bug, but was motivated by this testcase. It's tangential, and I just uploaded it for posterity. You can ignore it.

The actual fix for this bug lives in bug 760109. When it gets review, we can move forward.

Comment 12

[Andrew McCreight [:mccr8]]

Bug 760109 has landed now.  Can we confirm that it fixed this?

Comment 13

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

(In reply to Andrew McCreight [:mccr8] from comment #12)
> Bug 760109 has landed now.  Can we confirm that it fixed this?

Mariusz Mlynski, has the issue as you reported it been addressed?

Comment 14

[Mariusz Mlynski]

Attachment: Local file theft demo v2 (CRASHES)

(In reply to Al Billings [:abillings] from comment #13)
> Mariusz Mlynski, has the issue as you reported it been addressed?

I can't reproduce the testcases I posted in comment 0 and comment 6, the chrome objects such as Object or Function can't be accessed from the proto chain lookup anymore. However, the fix to bug 760109 is partially flawed -- the property lookup is not properly redirected to the local prototype chain. Thus, the inherited properties on COW come from the chrome counterparts of the objects in the prototype chain. Should I file a new bug for it?

Comment 15

[Bobby Holley (:bholley)]

Yeah, moz_bug_r_a4 filed this as bug 780370.

The patches as-landed are correct in their intended purpose, but I was incorrect in my assessment that they would fix this bug. I believe we need the patches in bug 778085 and bug 778086, which I'll try to get landed on m-c today assuming that Eddy is around.

Comment 16

[Bobby Holley (:bholley)]

Comment on attachment 638693 [details] [diff] [review]
Imply empty __exposedProps__ for Error objects. v1

Gah. So per bug 780370, the fix that we backported in bug 760109 is circumvented because Error objects don't have __exposedProps__.

This patch I uploaded here wasn't useful before (because callers could just do Object.getPrototypeOf and walk around the Error object). But now, this patch will plug the hole in a whack-a-mole sort of way. So maybe we should land this on m-a and m-b? It's only effective if there are no other ways that content can get its hands on a chrome object without __exposedProps__. I'm not sure how reasonable of an assumption that is. Blake?

Comment 17

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 638693 [details] [diff] [review]
Imply empty __exposedProps__ for Error objects. v1

Clearing the review request. I think this patch will be subsumed by the one that will come from our conversation today, right?

Comment 18

[Bobby Holley (:bholley)]

(In reply to Blake Kaplan (:mrbkap) from comment #17)
> Comment on attachment 638693 [details] [diff] [review]
> Imply empty __exposedProps__ for Error objects. v1
> 
> Clearing the review request. I think this patch will be subsumed by the one
> that will come from our conversation today, right?

Yep. I'll post that patch over in bug 780370.

Comment 19

[Alex Keybl [:akeybl]]

Let's wait for this to be fixed on trunk before tracking for a specific ESR version again.

Comment 20

[Alex Keybl [:akeybl]]

Same mistake here - 760109 has an ESR nomination, so this is still up for 15+.

Comment 21

[Alex Keybl [:akeybl]]

Can this bug be marked as fixed for branches and resolved now that 760109 has landed?

Comment 22

[Bobby Holley (:bholley)]

(In reply to Alex Keybl [:akeybl] from comment #21)
> Can this bug be marked as fixed for branches and resolved now that 760109
> has landed?

No, we also need the fix in bug 780370.

Comment 23

[Lukas Blakk [:lsblakk] use ?needinfo]

Updated flags on bug 780370 as well, we'll be looking to get this on Beta 16/ESR 10.0.8

Comment 24

[Daniel Veditz [:dveditz]]

bholley: is there an independent patch for this bug, or is it fully fixed by virtue of the two "depends on" bugs being fixed?

Comment 25

[Bobby Holley (:bholley)]

(In reply to Daniel Veditz [:dveditz] (mostly offline until Sept 4) from comment #24)
> bholley: is there an independent patch for this bug, or is it fully fixed by
> virtue of the two "depends on" bugs being fixed?

Should be fixed by those two, I believe. Bug 760109 has landed everywhere, bug 780370 still needs to land on beta and esr.

Comment 26

[u279076]

Can I get some guidance on how to use the PoC demos? I'm not able to reproduce any issues with the 2012-07-30 Firefox 17.0a1 debug build.

Comment 27

[Mariusz Mlynski]

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #26)
> Can I get some guidance on how to use the PoC demos? I'm not able to
> reproduce any issues with the 2012-07-30 Firefox 17.0a1 debug build.

Try to use Nightlies corresponding to dates when PoC's were posted. For example, local file theft demo v1 was posted on 2012-07-02, and it won't work with a 2012-07-30 build, because bug 760109 had been landed on 2012-07-28 and the bug I used to crash the browser was also fixed in the meantime.

To test the local file theft demos, simply open the page and wait. The browser will crash when session store collects form data, which is done periodically and may take anywhere from 1 to 20 seconds to happen. When the crash reporter window appears, click "Restart Firefox". If the browser doesn't crash for some reason, check the error console to see if there've been any errors -- if not, kill the browser manually.

The demos force the browser to fill the file input box with "C:\\secret.txt" -- this is a Windows path, so you may want to adjust it for your OS by opening the html file in a text editor, replacing this string with whatever references a file on your system, and saving the file.

Comment 28

[u279076]

Thanks Mariusz, with the first PoC and the Firefox 16.0a1 2012-06-25 build I get a very big context menu when I right click. Is this the effect of the PoC? In other words, a fixed build should not show the context menu?

Comment 29

[Mariusz Mlynski]

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #28)
> Thanks Mariusz, with the first PoC and the Firefox 16.0a1 2012-06-25 build I
> get a very big context menu when I right click. Is this the effect of the
> PoC? In other words, a fixed build should not show the context menu?

I first get an alert dialog saying "called" and then a big context menu as you described. A fixed build should not pop up the alert dialog and should display a normal context menu. I have no idea why you're not getting an alert dialog, perhaps the context menu immediately cancels the dialog on systems other than Windows? Anyway, that test was merely to show that the browser operates on a polluted XrayWrapper, and if the context menu doesn't look normal, that's an effect of calling bogus getSelection.

Comment 30

[u279076]

Sorry, I forgot to mention the alert dialog was showing before the context menu. In other words:

1. Load PoC testcase
2. Right click
> Alert box "called"
3. Click OK
> Large context menu appears

Comment 31

[u279076]

I managed to get the first local file demo to work by changing C:\\secret.txt to /home/ashughes/Downloads/foo.txt but the second local file demo does not work with the same change. Note, I used Firefox 16.0a1 2012-07-02 for testcase 1, Firefox 17.0a1 2012-08-08 for testcase 2.

Anything you can advise Mariusz?

Comment 32

[Mariusz Mlynski]

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #31)
> Anything you can advise Mariusz?

"Does not work" is a little vague. :-)

It doesn't crash? Wait 20 seconds, open the web console (Ctrl+Shift+K), type node and press enter. If it returns an input element, that means your browser somehow made it through the crash function -- kill it manually from the task manager and restart. If it returns undefined, I'm not sure what happens -- check the Error console (Ctrl+Shift+J).

It crashes, but the input box isn't filled on restart? You could try replacing |setTimeout('crash()',1000);| with |setTimeout('alert(1)',1000);| -- when the alert box appears, check the Error console. You could also try to open sessionstore.js located in your profile folder and search for the path you specified to see if it's been injected properly.

For the record, I've rechecked the second file theft demo in Firefox 17.0a1 2012-08-08 Win32 and it WFM.

Comment 33

[u279076]

1. Loaded testcase v2
2. Waited a minute
> No crash
3. Opened webconsole and typed "node" then pressed enter
> Returned [object HTMLInputElement @ 0x3d05490 (native @ 0x52bfd60)]
4. Killed the process and restarted Firefox
> Empty alert box appears
5. Dismiss the alert box
> Input box has the text

I get the following in error console:
Error: Exposing chrome JS objects to content without __exposedProps__ is insecure and deprecated. See https://developer.mozilla.org/en/XPConnect_wrappers for more information.
Source File: file:///home/ashughes/Downloads/file-theft-demo2.htm
Line: 24

I see the following in my sessionstore.js:
{"url":"file:///home/ashughes/Downloads/file-theft-demo2.htm","ID":3,"docshellID":6,"docIdentifier":3,"formdata":{"id":{"foo":{"type":"file","fileList":["/home/ashughes/Downloads/foo.txt"]}},"xpath":{}},"scroll":"0,0"}]

No crash though...

Again, using Firefox 17.0a2 2012-08-08 debug.

Comment 34

[Mariusz Mlynski]

OK, so the only thing that failed was the crash routine. I guess the vector I used is OS-specific. The crucial elements worked as expected, ie. the polluted function was called, the file path was injected into sessionstore.js and then "restored" after a restart. You can call it verified if you can't reproduce this behaviour in the latest Nightly.

Comment 35

[u279076]

> Andrew McCreight [:mccr8] 2012-09-07 08:58:17 PDT
> Keywords: verifyme
> Status: NEW → RESOLVED
> Resolution: --- → FIXED
> Last Resolved: 2012-09-07 08:58:17

I assume the target milestone is Firefox 18 based on the above. Updating the milestone and status flag. Please correct if this is in error.

Comment 36

[u279076]

Verified fixed with:
 * Firefox 18.0a1 2012-09-24 debug
 * Firefox 17.0a2 2012-09-24 debug
 * Firefox 16.0 2012-09-24 debug
 * Firefox 10.0.8esrpre 2012-09-24 debug