597162 - potential arbitrary code execution when type=content chrome frames web content

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[Daniel Veditz [:dveditz]]

Firefox 4 introduces about:addons -- the former add-ons dialog reborn as chrome-privileged content tab. If you select the "Get Addons" section you get a non-chrome frame hosting addons.mozilla.org. As bzbarsky notes in bug 593387 comment 29 this means web content can get a reference to a privileged chrome window through window.top

Can you do anything fun with this?

For testing purposes you can point extensions.webservice.discoverURL wherever you like (must be an https URL, although it can then navigate to non-SSL which is probably a bug).

If there are any new wrapper problems you can attack this way that would be good to know. 1) we may use this chrome-tab approach elsewhere, and 2) should AMO get hacked/defaced we don't want it to be able to escape into Firefox chrome.

Comment 1

[moz_bug_r_a4]

window.top is wrapped in a XOW.  The XOW seems to be safe, though content can
annoy a user by setting top.location.

But, if content accesses window.top via a XPCNativeWrapper or a SJOW, the
chrome window is wrapped in a COW.  And, since the chrome window does not have
__exposedProps__ property, content can access properties of the chrome window
via the COW.  And, about:addons page has a function that is abusable via the
COW.

Comment 2

[moz_bug_r_a4]

Attachment: testcase - arbitrary code execution

Comment 3

[Daniel Veditz [:dveditz]]

"sg:critical" may be a slight overstatement in practice. It's an accurate measure of the ultimate effects assuming other conditions that we don't yet know to be true: AMO getting hacked (specifically the page loaded by about:addons, not an arbitrary XSS) or us having another similar content-in-chrome-content situation. sg:very-nervous. I'll knock it down a notch due to mitigations, but we need to get on this.

And think hard about whether we have any other type=content chrome pages framing unprivileged content.

Should we reassign this to mrbkap now?

Comment 4

[Benjamin Smedberg]

Well, that depends on how we want to fix it. I think that we should probably fix this in docshell/dom by security-checking window.top. bz, does that make sense? Using the content/chrome docshell distinction to determine security accesses doesn't seem like a really good idea, since we've had problems with it in the past and aren't likely to get a UI-side solution any time soon, unless a docshell could dynamically switch between content-type and chrome-type.

Comment 5

[Boris Zbarsky [:bzbarsky]]

You mean change window.top to do the check that we recently made the X-Frame-Options stuff do?

We could probably do that, but it might break other things, of course (e.g. if any of these silly chrome-loading-in-content-docshell use window.top).  Also, it doesn't help this problem enough in the "hard" cases; see below.

> Using the content/chrome docshell distinction to determine security accesses
> doesn't seem like a really good idea

Well.... that's been the setup literally forever.  The problem is that people keep misusing it, as you point out.

> aren't likely to get a UI-side solution any time soon

Why not?  The only reason we don't have one yet is that we never made it a priority to have one, in spite of me pointing out for years (literally) that the way we load chrome things in tabs is broken by design and will bit us eventually security-wise.

> unless a docshell could dynamically switch between content-type and
> chrome-type.

That wouldn't obviously help us.

The testcase in comment 2 works if content script can get a reference to that chrome window in _any_ way, right?  Not just window.top?  In particular, if I call window.open and save the result in my evil.com page and then convince the user to navigate to about:addons in that resulting window, we get the same exploit, as far as I see.  The only way to prevent that problem is to make sure that we never load about:addons into any window that evil content may have a handle to or may get a handle to in the future.  In particular, just silently mutating the docshell type on navigation doesn't do what we want.  As long as we ever allow a docshell to contain a system-principal page _and_ to be reachable from non-chrome script (not necessarily at the same point in time!) evil script can get hold of the window.

Now maybe we can change some security check somewhere to make the particular exploit in comment 2 not work.  And maybe moz_bug_r_a4 can find another one.  But maybe we should just finally stop the whack-a-mole and use the completely functional security mechanism we already have in place and change the UI to not try load system-principal pages in content docshells, followed by enforcement on the core end to fail such loads if they're tried?

All a matter of priorities, I guess....

Comment 6

[Benjamin Smedberg]

You mean if I get a reference to a window (using window.open, say), and then the user navigates to about:config or something? I thought the inner window was designed to protect against this case. How is this different from navigating to any other unrelated domain which I'm not allowed to script?

I think our old definition of what makes a content/chrome docshell needs work. In my mind, the crucial difference is how it navigates and whether it can be targeted for loads. It's not primarily a distinction in terms of security, which should be handled through inner/outer windows, proxies, and such.

Comment 7

[Daniel Holbert [:dholbert]]

[The following is from bz -- I'm commenting on bz's behalf because his browser is being wonky]

> You mean if I get a reference to a window (using window.open, say), and then
> the user navigates to about:config or something?

Yes.

> How is this different from navigating to any other unrelated domain which I'm not allowed to script?

Because we special-case chrome (chrome-principal?  Not sure) stuff in XPConnect, apparently.  See comment 1: you don't get COWs for random domains.  If you think this is a COW bug, then Blake's your man.  Fwiw, I think that's likely what we need to fix here....

> In my mind, the crucial difference is how it navigates and whether it can be targeted for loads.

Note that we have different types of content windows depending on whether they can be targeted for loads.  The primary difference between chrome and content docshell is whether it's existence is visible to untrusted content at all or is just a browser implementation detail.

Comment 8

[Daniel Veditz [:dveditz]]

(In reply to comment #5)
> in spite of me pointing out for years (literally) that the way we load
> chrome things in tabs is broken by design and will bite us eventually
> security-wise.

Many years. bug 286651 was filed in 2005 and that was after a lot of discussion iirc. And even then we already had things depending on running chrome privileged code in a content window.

Comment 9

[moz_bug_r_a4]

(In reply to comment #5)
> The testcase in comment 2 works if content script can get a reference to that
> chrome window in _any_ way, right?  Not just window.top?  In particular, if I
> call window.open and save the result in my evil.com page and then convince the
> user to navigate to about:addons in that resulting window, we get the same
> exploit, as far as I see.

Yeah, that way chrome pages are exploitable.

Comment 10

[moz_bug_r_a4]

Attachment: testcase 2 - arbitrary code execution

The following pages are exploitable:
about:about, about:addons, about:config, about:crashes, about:memory,
about:plugins, about:privatebrowsing, about:sessionrestore, about:support,
about:sync-tabs

Comment 11

[chris hofmann]

probably need to have mrbkap or jst to pick up ownership of this bug at this point.  who can have a look?

Comment 12

[Damon Sicore (:damons)]

Assigning to jst.

Comment 13

[moz_bug_r_a4]

Since compartments has landed, the old testcases no longer work.  I'll attach
new testcases.

Comment 14

[moz_bug_r_a4]

Attachment: testcase 3 (modified testcase 1)

This uses the Error Console instead of the Web Console due to bug 604722.

Comment 15

[moz_bug_r_a4]

Attachment: testcase 4 (modified testcase 2)

Comment 16

[Johnny Stenback (:jst)]

Blake, this would be fixed if we make chrome native's default to safe, you had a plan to do that, right?

Comment 17

[Blake Kaplan (:mrbkap) (inactive)]

Yes.

Comment 18

[Johnny Stenback (:jst)]

Blake has a plan, he gets to own this.

Comment 19

[Johnny Stenback (:jst)]

Seems like this is still a problem even after bug 572647 got fixed, the second testcase still appears to work here.

Comment 20

[moz_bug_r_a4]

If possible, could someone please cc me on bug 572647?

Comment 21

[Boris Zbarsky [:bzbarsky]]

Done.

Comment 22

[(no longer active)]

Is there a good reason that this blocks betaN?  If not, it should be moved over to final+.

Comment 23

[Mike Beltzner [:beltzner, not reading bugmail]]

Preeeetttty invasive code would make me feel better with beta feedback, but I agree that this can be a final blocker, especially if it's a soft blocker.

Comment 24

[Mike Beltzner [:beltzner, not reading bugmail]]

** PRODUCT DRIVERS PLEASE NOTE **

This bug is one of 7 automatically changed from blocking2.0:final+ to blocking2.0:.x during the endgame of Firefox 4 for the following reasons:

 - it was marked as a soft blocking issue without a requirement for beta coverage

Comment 25

[Daniel Veditz [:dveditz]]

mrbkap: any ETA on this? Should someone else own the fix (and if so who?)?

Comment 26

[Christopher Blizzard (:blizzard)]

mrbkap, looking for an ETA here as this is tracking for Firefox 5.

Comment 27

[Asa Dotzler [:asa]]

With no patch and the suggestion that this change could be "invasive" I'm concerned that this is not likely to make the Firefox 5 train.  Are we OK with that?

Comment 28

[Boris Zbarsky [:bzbarsky]]

The patch in bug 641342 fixes this bug and is landed on TM.  It'll be on m-c after the next TM merge...

Comment 29

[chris hofmann]

moz_bug_r_a4,  can you grab a tracemonkey build and try to confirm this bug is fixed there and is free of possible side effects.  It would be good to land the fix on mozilla-central and try to get in fx5 beta if all looks well.

Comment 30

[moz_bug_r_a4]

It seems that the patch in bug 641342 is already landed on mozilla-central.

http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=673f93bb84aa

I confirmed that this bug is fixed on tracemonkey and mozilla-central.

Comment 31

[Johnny Stenback (:jst)]

This is fixed then. But not for Firefox 5 yet.

Comment 32

[Asa Dotzler [:asa]]

moving the Firefox 5 tracking over to the bug with the patches.

Comment 33

[Daniel Veditz [:dveditz]]

The fix for bug 641342 has landed in beta and does fix testcases 3 and 4