286651 - Strip privilege from chrome loaded in a browser/content window

Status: NEW

(Core :: Security, enhancement)

Description

[Daniel Veditz [:dveditz]]

To prevent future exploits we need to strip elevated privileges from content
displayed in a browser window (e.g. chrome: urls). We've had a series of
exploits recently and over the years that take advantage of loading bits of
Mozilla chrome in a window or frame and then leveraging that into a privilege
escalation exploit.

This is going to be unpopular with some folks (sorry, Ben). We should support a
switch along the lines of the signed.applets.codebase_principal_support pref for
these folks.

We also have to move any special about: content that requires privileges into
its own window. "about:config" is the obvious one (see the aboutconfig
Thunderbird extension for how this would look--works fine for this ultimate Geek
tool). "about:plugins" also has chrome privileges but I'm not sure why -- on the
surface it looks like all the information it needs is available from the public
navigator.plugins object.

Comment 1

[Daniel Veditz [:dveditz]]

Clearing the confidential flag, this isn't an exploit per se and in any case not
unknown to attackers.

Comment 2

[Boris Zbarsky [:bzbarsky]]

> "about:plugins" also has chrome privileges but I'm not sure why

Because it's localized, so needs access to XPCOM objects (stringbundles).

Perhaps we can use a DTD or something here instead?

Comment 3

[Benjamin Smedberg]

Why into its own window instead of some other solution (for instance, allow a
<xul:browser type="chrome"> in the main <tabbrowser> widget.

Comment 4

[Robert Kaiser]

(In reply to comment #2)
> > "about:plugins" also has chrome privileges but I'm not sure why
> Because it's localized, so needs access to XPCOM objects (stringbundles).
> Perhaps we can use a DTD or something here instead?

When fixing bug 56863 I needed to open that potential security issue (and
mstoltz as one of the security people was aware of that). I'm preaching about
this problem every year in my FOSDEM talks since then :)

The problem is that about:plugins is being generated by JS code, and for doing
L10n of JS in a sensible way, you need to access stringbundles. We can't access
stringbundles without chrome privileges though, and that the real bug here - see
bug 98298 - but noone seemed to care up until now.

I don't think having access to stringbundles would be a security issue itself
(provided we check the bundle's location, so that e.g. about:plugins might need
the stringbundle living in about: "protocol"), but it needs to be done to be
able to remove chrome privileges from about:plugins (this might benefit others
as well, but that's subject to bug 98298 and doesn't need to be discussed here).

Comment 5

[Ben Bucksch (:BenB)]

If you implement this, please do not consider the issue to be resolved, but keep
looking for solutions. We are still vulnerable to the same problem in other
places, as outlined in my mails to security-group, which is why disagreed with
this solution.

> This is going to be unpopular with some folks (sorry, Ben). We should
> support a switch along the lines of the
> signed.applets.codebase_principal_support pref for these folks.

I don't think I'd want a special flag. You'd probably worry much less about
these types of exploits, which means I'd be more vulnerable than I am now.

FYI: A simple case where I need chrome URLs in the browser iframe is a search
feature that appears in the main pane, similar to search engine result pages,
but I need chrome rights.

> allow a <xul:browser type="chrome"> in the main <tabbrowser> widget

That's the workaround I thought of, too. However, an extra attribute + existing
arc attribute probably has the probelm of a race condition vulnerability: I set
the chrome attribute, then set the src attribute, then the iframe clears its
chrome attribute (otherwise we may have a similar vulnerability as today), but
we are still vulnerable in the time between I set the type=chrome and the src
attribute. So, I'd make the src attribute only ever accept "safe" URLs and add a
special *method* to the iframe which would be only callable by chrome and the
only way to set it to a chrome URL:
document.getElementById("mybrowser").loadChrome("chrome://foo/content/");.

> We also have to move any special about: content that requires privileges
> into its own window.

I think that's a good idea. After all, most of this stuff *is* part of the
application, not a page, and belongs into the browser UI, not the browser
content window. It was a cute idea back in old Netscape times.

Comment 6

[Robert Kaiser]

(In reply to comment #5)
> > We also have to move any special about: content that requires privileges
> > into its own window.
> 
> I think that's a good idea. After all, most of this stuff *is* part of the
> application, not a page, and belongs into the browser UI, not the browser
> content window. It was a cute idea back in old Netscape times.

Right. Be aware that this doesn't really resolve the issue we have with
about:plugins though. Some plugins do rely on us displaying an HTML page [sic]
that contains their description property (navigator.plugins[i].description), as
they don't only send some HTML in that property but even embedded data for
plugin prefs (via an <object>, I know it sounds insane but it's reality). Even
if we show that HTML in an own window, those descriptions are still shown with
chrome privs :( - see those about:plugins bugs for more...

For everything else, this sounds good though.

Comment 7

[James Ross]

If I understand what is going on here right, it would basically be the end of
ChatZilla [1], unless the XUL container can do something special to let the
content it loads keep its permissions.  I'm also not sure how this would affect,
say, the preference panels in Mozilla itself... that would need to be 'fixed',
wouldn't it?

[1] ChatZilla has a <browser> for each view, which loads a chrome: HTML page,
and most definately needs its chrome permissions to actually work.

Comment 8

[Jesse Ruderman]

This would break my Thumbs extension :(  http://www.squarefree.com/extensions/thumbs/

Comment 9

[Robert Pendell]

I concur that if this bug gets resolved to put into consideration what other extensions may be doing.  In particular FireFTP and ChatZilla.

Comment 10

[Robert James Gordon-Rawlings]

So what I'm getting from this is that you want to cripple a large swathe of current content delivery methodology and part of the most powerful functionality of the chrome engine in order to prevent... what, future exploits that may or may not come into existance? And all you're doing to prevent these exploits is quite literally disable functionality and flexibility?
I'd buy that for a dollar. Not.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Robert, I suspect you pretty much completely misunderstood what this bug is about.  I suggest taking your concerns to the security newsgroup, where I can probably explain at length to you exactly what's being proposed here and we can talk about your use cases and how they can be best addressed.

Comment 12

[Samuel Bronson]

Is this still planned?

Because it seems like about:preferences and about:addons have gone in the wrong direction for that, if I'm not totally confused.

Also, about:plugins started needing more privileges for its functionality (not just l10n) back in bug #757726 because navigator.plugins wasn't going to enumerate all plugins anymore, and even though *that* filtering was backed out in bug #1169945, navigator.plugins still doesn't list disabled plugins (or click-to-play "Flash" plugins, since bug #1186948 ?), so going back to the old way would make it much less useful.

And honestly most of the about: pages probably need privileges, except pages like about:, about:mozilla, about:credits, about:robots, about:logo, or about:blank that are basically just for entertainment or informational purposes.

Comment 14

[:Gijs (he/him)]

bz and I talked about this for a bit in bug 1296936.

(In reply to Samuel Bronson from comment #12)
> Because it seems like about:preferences and about:addons have gone in the
> wrong direction for that, if I'm not totally confused.

There are two ways to avoid the issue that these pages need privileges:

1) make the page function without privileges
2) make the page load in a type=chrome docshell instead.

At this stage, I *think* all the chrome-privileged about: pages are set up to load in a non-remote docshell, and all/most of the other ones load in a remote ones. So the simplest solution might be to enforce that the browser type is chrome for the case where we load a chrome-privileged page. Because we already have a bunch of infrastructure for swapping docshells from remote to non-remote and vice versa, I think we can leverage this to get to option (2) with comparatively little work. Mike, does that sound plausible or am I missing something substantial here?

(To be clear, I explicitly don't really care about loading chrome:// things in a tab. If we break add-ons that way, so be it. We should announce that ahead of time, but with webextensions we're going this way anyway so it shouldn't be a big surprise or impediment to having in-content pages for add-ons that are still maintained.)

If not, I think the time is ripe to actually do this. Doing so will require e10s to be enabled, but considering that's where we're heading anyway, I don't think that's a big deal.

> Also, about:plugins started needing more privileges for its functionality
> (not just l10n) back in bug #757726 because navigator.plugins wasn't going
> to enumerate all plugins anymore, and even though *that* filtering was
> backed out in bug #1169945, navigator.plugins still doesn't list disabled
> plugins (or click-to-play "Flash" plugins, since bug #1186948 ?), so going
> back to the old way would make it much less useful.

Plugins are going away, so I'm not sure how bad this particular issue really is.

But generally, for pages like about:plugins or about:support, the simple solution is using a frame script to inject the privileged content / interactions. Pages like about:neterror already do this. This would be difficult for about:addons and about:preferences because of their complexity and use of XBL bindings and the like, but much less so for about:support, about:plugins, and other reasonably small pages. It might be worth pursuing as part of an internship or general hardening of the codebase. It can be done independently of the idea I suggested above.

Comment 15

[Mike Conley (:mconley) (:⚙️)]

(In reply to :Gijs Kruitbosch from comment #14)
> There are two ways to avoid the issue that these pages need privileges:
> 
> 1) make the page function without privileges
> 2) make the page load in a type=chrome docshell instead.
> 
> At this stage, I *think* all the chrome-privileged about: pages are set up
> to load in a non-remote docshell, and all/most of the other ones load in a
> remote ones. So the simplest solution might be to enforce that the browser
> type is chrome for the case where we load a chrome-privileged page. Because
> we already have a bunch of infrastructure for swapping docshells from remote
> to non-remote and vice versa, I think we can leverage this to get to option
> (2) with comparatively little work. Mike, does that sound plausible or am I
> missing something substantial here?
> 

There are nsIAboutModule flags that can be used to dictate which process an about: page should be loaded into. See [1].

Most privileged about: pages are still loaded in the parent process. A few have been migrated (about:plugins, for example, which uses RemotePageManager.jsm).

Regarding the type="chrome" requirement - I suspect that we can probably make this work, but it might not be as simple as it sounds. As I understand it, here's how it currently works:

1) DocShell attempts to load a URI that it cannot load in its current process
2) Message is sent to the parent window browser.js to "RedirectLoad", which calls LoadInOtherProcess.
3) LoadInOtherProcess has SessionStore scoop up the state of the browser, and then call "restoreTab" on that tab
4) restoreTab calls restoreTabContent, since we're loading right away, and that calls updateBrowserRemotenessByURL here: http://searchfox.org/mozilla-central/rev/95b0ecf4b59a01e0524ca02f6c96ecaabe38f4d5/browser/components/sessionstore/SessionStore.jsm#3384

It's in step 4 that you _might_ want to set the attribute as well. Based on the URL, we might want to tell updateBrowserRemoteness to set that attribute before re-inserting the <xul:browser> into the DOM. That's what I'd start looking at, anyhow.

I hope that helps!

[1]: https://developer.mozilla.org/en-US/Firefox/Multiprocess_Firefox/Which_URIs_load_where

Comment 16

[:Gijs (he/him)]

So I tried poking at this today. I'm running into 2 issues so far:

1) docshell and various other things like asking for, effectively, the "same type top/root" item in the docshell tree for things like session history and various other checks. If the docshell type of browsers that load e.g. about:preferences is "chrome", that breaks because now it asks for the root (ie browser.xul). That's not what we want (e.g. we don't want the back/fwd buttons in the browser to try to navigate browser.xul to somewhere else). I don't think we want to add another type of docshell (after chrome/content) and so I'm looking for some kind of other logic to make the tree traversal not consider the toplevel browser.xul and this XUL <browser> "same type". The docshell tree traversal already specialcases mozbrowser and app frames, which seems to be based on the frameType in the docshell. Would it make sense to add an attribute to XUL browsers that imply a different frame type when we create a docshell for such browsers? What would be the best way of doing this?
(Because about:preferences, for instance, creates iframes that load other privileged documents, I don't think treating every chrome type shell as its own root is wise.)


2) AFAICT we don't create session / global history for chrome type docshells (certainly, asking for the nsIWebNavigation's canGoback/canGoForward and various other things throws awkward NS_ERROR_UNEXPECTED and the like) . Am I missing something? What would break if we started doing that (and where would that need to happen) ?

Comment 17

[Boris Zbarsky [:bzbarsky]]

> Would it make sense to add an attribute to XUL browsers that imply a different frame type when we create a docshell for such browsers?

Yes, that seems plausible.  Basically have an explicit boundary rather than an implicit one where the docshell type changes.

> AFAICT we don't create session / global history for chrome type docshells

This is conditioned on mIsTopLevelContent in nsFrameLoader::MaybeCreateDocShell.  That comes from nsFrameLoader::AddTreeItemToTreeOwner which will return true if (aParentType == nsIDocShellTreeItem::typeChrome && isContent).  Presumably we would change this code to also do it in the new special case we introduce.

We have so much complexity around this stuff.  :(