1296936 - Stop supporting loading chrome:// and/or privileged URIs in browser tabs

Status: RESOLVED DUPLICATE of bug 286651

(Firefox :: Tabbed Browser, enhancement)

Description

[Fanolian]

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0
Build ID: 20160821030226

Steps to reproduce:

1. In a new profile, open one tab and only in one window. Go to about:memory.
2. Minimize memory usage a few times then measure a verbose report.
3. Search for "detached" (without quotes) in about:memory. It should have no detached windows at this point.

4. Open a new tab (not window). Open chrome://browser/content/browser.xul, and then close it.
5. Go back to about:memory, minimize memory usage and measure again. An instance of this is created:
top(none)/detached/window(chrome://browser/content/browser.xul)/dom/event-listeners

6. Repeat step 4 and 5 a few times. More top(none)/detached/… are created and cannot be destroyed unless restarting browser.

Comment 1

[Fanolian]

:bkelly, sorry for bothering you.

I see you reporting a few leaks about browser.xul recently (bug 1296094, bug 1286264, bug 1277358, bug 1277360, and bug 1296099). Is this bug a dupe of one of your reports?

(I know my STR is not a normal browsing scenario. But a leak is a leak.)

Comment 2

[Fanolian]

s/bug 1286264/bug 1283264
s/bug 1277360/bug 1277630

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

To be honest, I'm shocked we support typing chrome:// URLs in the URL bar.  Boris, does this sound right to you?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Which, being shocked?  I'm not shocked, but I do think we should stop allowing it!

Of course I think we should stop allowing loading system-principal stuff in non-chrome docshells, period, but that's a much bigger change.  Stopping chrome:// stuff via the URL bar would be fairly simple in the URL bar code, I'd think.

Comment 5

[:Gijs (he/him)]

(In reply to Boris Zbarsky [:bz] from comment #4)
> Which, being shocked?  I'm not shocked, but I do think we should stop
> allowing it!
> 
> Of course I think we should stop allowing loading system-principal stuff in
> non-chrome docshells, period, but that's a much bigger change.

I think this would be a good idea, but as you say, that's a big fish to fry. We'd also likely trade bugs of kind X for bugs of kind Y (that is, we'd need listeners from chrome code that listen for events from content-privileged "about" and other pages, and act on them, and the result would be that we'd just have leaks / security issues in the chrome-privileged "listening" code instead.)

>  Stopping
> chrome:// stuff via the URL bar would be fairly simple in the URL bar code,
> I'd think.

Sure, but there'd be a million and one ways to bypass it (bookmarks, drag and drop, ...). There isn't really a single codepath that loads URIs into content docshells. So I don't really know why we would bother writing code specific to the URL bar for this. What would be the point?

Also, it would break figuring out what's busted when working on the browser. If I write code with JS errors, the browser comes up but the browser console menu (and shortcut) don't exist. Right now I can type chrome://global/content/console.xul in the address bar and still see errors. There's usually no stderr/stdout equivalent (definitely not on Windows) so I'd be having to bisect my code to find out where I've made a mistake. That doesn't sound like a lot of fun. (If you don't write frontend JS, imagine your application crashes but you have no gdb/lldb/whatever, no return value indicating what kind of crash it was, and no stack trace.)

And finally it'd likely break some add-ons that open such URLs to provide their users with ways to manage settings etc.

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

Since this is a browser.xul issue I'm going to move it to the firefox component.  I don't think we should support this at all, but I will leave it to that team to decide.

Comment 7

[:Gijs (he/him)]

(In reply to Ben Kelly [:bkelly] from comment #6)
> I don't think we should support this at all, but I will leave it
> to that team to decide.

It's not "supported", it's just "something you can do today". Making it fully impossible is hard, see comment #4 / comment #5. I don't want to wontfix this, but the alternative is letting it sit in limbo for the foreseeable future. Certainly the leak is WONTFIX, so I'm just going to morph the bug.
 
(In reply to :Gijs Kruitbosch from comment #5)
> (In reply to Boris Zbarsky [:bz] from comment #4)
> > Which, being shocked?  I'm not shocked, but I do think we should stop
> > allowing it!
> > 
> > Of course I think we should stop allowing loading system-principal stuff in
> > non-chrome docshells, period, but that's a much bigger change.
> 
> I think this would be a good idea, but as you say, that's a big fish to fry.
> We'd also likely trade bugs of kind X for bugs of kind Y (that is, we'd need
> listeners from chrome code that listen for events from content-privileged
> "about" and other pages, and act on them, and the result would be that we'd
> just have leaks / security issues in the chrome-privileged "listening" code
> instead.)

One more thought about this: we could disable loading system-principal stuff in non-chrome docshells more easily if we swapped the docshells with chrome ones for the system-principal'd pages that we do want to load (like, I dunno, the preferences, or about:memory, about which for some reason there is no shock/outrage even though it's basically the same thing).

To me though, that seems scarier still. Is that just me? Boris, thoughts?

Comment 8

[Boris Zbarsky [:bzbarsky]]

> To me though, that seems scarier still.

In general, I would agree, which is why it has not happened yet.  But don't we already do something like that when you go from something that gets loaded in the content process to something that gets loaded in the chrome process?  Can we leverage some of that infrastructure?

In general, I think there are several issues here, which are not necessarily related:

1)  Should we allow loads of system-principal stuff in non-chrome docshells?  Ideally, the answer would be "no" for basic security reasons, and this would apply to preferences, about:memory, whatever.  dveditz and I have been pushing for that for literally over a decade.

2)  Should we allow loading a chrome:// URI (as opposed to an about: URI which redirects to chrome://) in a browser tab, whether it's using a content docshell or a chrome docshell?  For about: we generally design them to work in that situation, but general chrome:// things are not thus designed (and in fact may only function correctly when loaded in particular ways; e.g. viewsource.xul required certain dialog arguments last I checked).  As you note, disallowing this across the board may break some extensions, so if we did that we'd probably have to make it opt-in or something and opt most (all?) of our chrome in.

3)  If we allow loading browser.xul in a tab, but ensure that it gets a chrome docshell, does that actually fix the leak?  It might, or it might not.

Comment 9

[:Gijs (he/him)]

(In reply to Boris Zbarsky [:bz] from comment #8)
> > To me though, that seems scarier still.
> 
> In general, I would agree, which is why it has not happened yet.  But don't
> we already do something like that when you go from something that gets
> loaded in the content process to something that gets loaded in the chrome
> process?  Can we leverage some of that infrastructure?

Maybe/probably. But I am at least as worried about the security implications as by the difficulty of implementing this.

Right now, if you're a content page and you through magical means manage to escalate to a chrome:// or about: page, at least the content-type of the docshell is still some level of security (or is it? Maybe I'm putting hope in things I shouldn't?). If we then automatically start transferring you to a different docshell that seems like that layer of security goes away immediately.

> In general, I think there are several issues here, which are not necessarily
> related:
> 
> 1)  Should we allow loads of system-principal stuff in non-chrome docshells?
> Ideally, the answer would be "no" for basic security reasons, and this would
> apply to preferences, about:memory, whatever.  dveditz and I have been
> pushing for that for literally over a decade.

I'm on board with this if the "proxy into chrome type docshells where we need it" solution is acceptable - it's just work. We'd need to prioritize it happening, which might be difficult if it doesn't solve a user-centered problem. That or someone else from outside the frontend needs to volunteer to write the patches...

> 2)  Should we allow loading a chrome:// URI (as opposed to an about: URI
> which redirects to chrome://) in a browser tab, whether it's using a content
> docshell or a chrome docshell?  For about: we generally design them to work
> in that situation, but general chrome:// things are not thus designed (and
> in fact may only function correctly when loaded in particular ways; e.g.
> viewsource.xul required certain dialog arguments last I checked).  As you
> note, disallowing this across the board may break some extensions, so if we
> did that we'd probably have to make it opt-in or something and opt most
> (all?) of our chrome in.

We could probably do another chrome manifest flag or something? But that would best be a restriction implemented with some help from core, rather than by trying to gatekeep all the access points (bookmarks, url bar, what have you).

I'm not sure what the (security) benefit of doing this would be, though. Can you elaborate on what you think we would gain from doing this, besides "not get bugreports like this" ? And if we think there is a (security) benefit, shouldn't we want add-ons to adopt it and/or have to opt out (viz. xpcnativewrappers=no etc.) ?

> 3)  If we allow loading browser.xul in a tab, but ensure that it gets a
> chrome docshell, does that actually fix the leak?  It might, or it might not.

I think it's wontfix either way. Loading browser.xul in a tab is an accident of how URLs and our internal architecture work. There's no real reason to do it, we don't support it, and I basically don't care whether or to what degree it works, as it tells us approximately nothing about whether it'd work if you opened it in a window.

Comment 10

[Boris Zbarsky [:bzbarsky]]

> Maybe I'm putting hope in things I shouldn't?

Maybe.  If you can control the script in that system-principal page enough to do a window.open of your choice, for example, you can just open a chrome docshell.

> But that would best be a restriction implemented with some help from core

Yes, if we decide to take that approach I agree.

> I'm not sure what the (security) benefit of doing this would be, though.

A basic decrease in "supported" configurations.  Right now we have 4 "supported" configurations: trusted (i.e. system principal) stuff in a chrome docshell, trusted stuff in a content docshell, untrusted stuff in a content docshell, untrusted stuff in a chrome docshell.  In an ideal world we would only support trusted stuff in chrome docshells and untrusted stuff in content docshells.  The latter is just as important as the former, btw; right now an extension can load stuff over http:// into a chrome iframe in the UI and then that untrusted stuff can navigate window.top, which is the browser chrome itself...

> shouldn't we want add-ons to adopt it and/or have to opt out

Yes.

Comment 11

[:Gijs (he/him)]

(In reply to Boris Zbarsky [:bz] from comment #10)
> > Maybe I'm putting hope in things I shouldn't?
> 
> Maybe.  If you can control the script in that system-principal page enough
> to do a window.open of your choice, for example, you can just open a chrome
> docshell.
> 
> > But that would best be a restriction implemented with some help from core
> 
> Yes, if we decide to take that approach I agree.

How do we get to a decision? :-)

> > I'm not sure what the (security) benefit of doing this would be, though.
> 
> A basic decrease in "supported" configurations.  Right now we have 4
> "supported" configurations: trusted (i.e. system principal) stuff in a
> chrome docshell, trusted stuff in a content docshell, untrusted stuff in a
> content docshell, untrusted stuff in a chrome docshell.  In an ideal world
> we would only support trusted stuff in chrome docshells and untrusted stuff
> in content docshells.  The latter is just as important as the former, btw;
> right now an extension can load stuff over http:// into a chrome iframe in
> the UI and then that untrusted stuff can navigate window.top, which is the
> browser chrome itself...

Right, but you're describing the benefit of not loading trusted things in untrusted docshells or vice versa. I was specifically interested in the benefit of restricting the load of "useless" chrome:// URIs but leaving e.g. about:memory, which is how I understood item (2) in comment #8. If we keep about: ones, "just" not loading chrome:// ones doesn't seem to me as if it makes much of a difference from a security perspective - but maybe I'm missing something?

Comment 12

[Boris Zbarsky [:bzbarsky]]

> Right, but you're describing the benefit of not loading trusted
> things in untrusted docshells or vice versa.

Yes, that's what I thought we were talking about, and the thing that dveditz and I have been asking for for a while.

> I was specifically interested in the benefit of restricting
> the load of "useless" chrome:// URIs but leaving e.g. about:memory,
> which is how I understood item (2) in comment #8.

Yeah, I thought we were talking about item (1).  Item (2) only has security implications in the sense that it would disallow chrome stuff that doesn't expect to be running in a content-type docshell from doing so.  Things like about:memory _do_ expect to be running in that environment and are tested in it, and hence there is slightly less chance of them doing something wacky in that environment...

Comment 13

[:Gijs (he/him)]

(In reply to Boris Zbarsky [:bz] from comment #12)
> > Right, but you're describing the benefit of not loading trusted
> > things in untrusted docshells or vice versa.
> 
> Yes, that's what I thought we were talking about, and the thing that dveditz
> and I have been asking for for a while.
> 
> > I was specifically interested in the benefit of restricting
> > the load of "useless" chrome:// URIs but leaving e.g. about:memory,
> > which is how I understood item (2) in comment #8.
> 
> Yeah, I thought we were talking about item (1).  Item (2) only has security
> implications in the sense that it would disallow chrome stuff that doesn't
> expect to be running in a content-type docshell from doing so.  Things like
> about:memory _do_ expect to be running in that environment and are tested in
> it, and hence there is slightly less chance of them doing something wacky in
> that environment...

OK. In that case, I don't think there's much to gain from that. "Don't do that" rather than hardcoded restrictions seems OK to me.

Comment 14

[Ben Kelly [:bkelly, not reviewing]]

(In reply to :Gijs Kruitbosch from comment #13)
> OK. In that case, I don't think there's much to gain from that. "Don't do
> that" rather than hardcoded restrictions seems OK to me.

Are you sure?  There is already a problem with users following malicious "this one weird trick" directions to do bad things to their browsers.  For example, see the loud warnings about pasting into devtools that facebook puts in the web console and our block against pasting into web console.  Saying "just don't paste chrome:// URLs into the bar" is not really going to help users who don't understand what is going on.

Comment 15

[:Gijs (he/him)]

(In reply to Ben Kelly [:bkelly] from comment #14)
> (In reply to :Gijs Kruitbosch from comment #13)
> > OK. In that case, I don't think there's much to gain from that. "Don't do
> > that" rather than hardcoded restrictions seems OK to me.
> 
> Are you sure?  There is already a problem with users following malicious
> "this one weird trick" directions to do bad things to their browsers.

But nobody has pointed out anything *malicious* (rather than just broken/unfortunate) that can happen if you do this. The bug got filed for a leak, not a phishing/identity-theft attack. It's hard for me to see how that'd be done any quicker by loading an obscure URI via the location bar than by e.g. using whatever keyboard shortcut would normally bring up that window, or using about:config or about:debugging to enable remote control of the instance, none of which would be prevented by just disabling the loading of chrome:// URIs in content tabs.

As said, I'm happy to help pursue (1) from comment #8, which as a side effect will likely end up preventing this - but that change has security benefits, and just blocking chrome://* doesn't.

Comment 16

[Dão Gottwald [:dao]]

(In reply to Boris Zbarsky [:bz] from comment #8)
> 1)  Should we allow loads of system-principal stuff in non-chrome docshells?
> Ideally, the answer would be "no" for basic security reasons, and this would
> apply to preferences, about:memory, whatever.  dveditz and I have been
> pushing for that for literally over a decade.

There's probably already a bug filed on this somewhere, isn't there? We should probably mark this a duplicate of that, or wontfix this for the reasons Gijs mentioned.