Last Comment Bug 429070 - exposing Components.interfaces to untrusted content leaks information about installed extensions
: exposing Components.interfaces to untrusted content leaks information about i...
Status: RESOLVED FIXED
[sg:low][tor]
: addon-compat, privacy, sec-low
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: unspecified
: All All
: -- normal with 15 votes (vote)
: mozilla22
Assigned To: Camilo Viecco (:cviecco)
:
Mentors:
javascript:(function(){var o=[];for(v...
Depends on: 462483 726053 790732 808457
Blocks: abp
  Show dependency treegraph
 
Reported: 2008-04-14 20:08 PDT by James Justin Harrell
Modified: 2015-01-09 21:10 PST (History)
52 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
proposed patch (13.39 KB, patch)
2011-07-25 14:06 PDT, arno renevier
mrbkap: review+
Details | Diff | Splinter Review

Description James Justin Harrell 2008-04-14 20:08:06 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.8.1.13) Gecko/20080325 Ubuntu/7.10 (gutsy) Firefox/2.0.0.13
Build Identifier: 

The Components.interfaces object is a collection of XPCOM interfaces, including interfaces defined by extensions. Because it is exposed to untrusted content (e.g. web pages), it can be used without knowledge or consent to obtain information about visitors that the visitors may not want known. For example:

if( "nsIFireBug" in Components.interfaces ) {
    alert("Your a hacker, I called the police!");
}

Reproducible: Always

Steps to Reproduce:
1. Install an extensions that defines an XPCOM interface.
2. Test for the existence of a property on Components.interfaces with the same name as an interface defined by the extension.
Actual Results:  
The existence of the interface is exposed to untrusted code.

Expected Results:  
The existence of the interface is not exposed to untrusted code.
Comment 1 Wladimir Palant 2008-04-14 23:05:53 PDT
Adblock Plus had to remove its XPCOM interface in version 0.7.5.2 because of this issue - now extensions communicating with Adblock Plus are supposed to use wrappedJSObject.
Comment 2 Alex Vincent [:WeirdAl] 2008-04-14 23:22:36 PDT
What would the proposed solution be?  Making extension interfaces be non-enumerable in general, or just from content?  Is that even possible?

This is exceptionally late in the game for 1.9... 
Comment 3 Wladimir Palant 2008-04-15 00:12:54 PDT
Alex, the issue is not only enumeration. If you disallow enumeration, web sites will check explicitly for |typeof Components.interfaces.nsIFireBug|. So I see only two real solutions:

1. Make access to Component.interfaces require privileges (don't think we want to do that, remote XUL needs it).
2. Don't expose some of the interfaces depending on callers privileges (probably difficult to implement, unclear how to decide which interfaces should be "public").

Either way, this isn't for 1.9.
Comment 4 alex_mayorga 2008-12-08 10:07:45 PST
What can I do to confirm this one?
Comment 5 Wladimir Palant 2008-12-08 10:35:30 PST
Moving to correct component (from what I can tell) and confirming.
Comment 6 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2008-12-08 10:43:23 PST
Why does remote XUL need access to Components.interfaces? for accessibility stuff?
Comment 7 Wladimir Palant 2008-12-08 11:21:52 PST
To implement XPCOM components (QueryInterface). At the very least it was necessary to implement nsITreeView in remote XUL but now that is forbidden anyway.
Comment 8 Mike Perry 2010-06-28 03:45:18 PDT
Gregory Fleischer demonstrated at defcon last year that these interfaces can also be used to fingerprint Firefox down the to the minor version:
http://pseudo-flaw.net/tor/torbutton/fingerprint-firefox.html

Note that his test has not been updated since 3.5.3, hence it reports 3.5.3 for more recent Firefoxes.

Adding this note because Bug 572659 aims to remove the patch-level from the UI string (which I think is a good idea to help frustrate drive-by-download attacks and improve privacy, but is ultimately pointless if patch-level can still be determined from this data).
Comment 9 Wladimir Palant 2010-06-28 04:09:48 PDT
This PoC actually uses a relatively "stupid" approach by only checking the interface existence - it could be made more precise by checking the UUID associated with the interface as well (like Components.interfaces.xpcIJSWeakReference.number). That would allow not only detecting interface addition/removal but also specific changes to interfaces.

However, bug 572659 has bigger problems IMO as I noted in bug 572659 comment 3.
Comment 10 shawn.sumin 2011-03-31 15:51:01 PDT
The Tor Project / Electronic Frontier Foundation is paying to have this bug fixed.

"If you know C++ and/or Firefox internals, we should be able to pay you for your time to address these issues and shepherd the relevant patches through Mozilla's review process."

Source: https://blog.torproject.org/blog/web-developers-and-firefox-hackers-help-us-firefox-4
Comment 11 Alex Vincent [:WeirdAl] 2011-03-31 16:06:45 PDT
(In reply to comment #3)
> So I see only two real solutions:
> 
> 1. Make access to Components.interfaces require privileges (don't think we want
> to do that, remote XUL needs it).

Remote XUL is dead in FF4, so this option is viable again.

That said, I'm slightly uneasy about it.  On the one hand, it helps to know what an object implements.  On the other, Components.interfaces shouldn't be necessary to do that.  (obj instanceof Range, obj instanceof Window, obj instanceof Node, obj instanceof HTMLInputElement, etc.)

Now, I haven't done much web development in a long time, so maybe I'm overstating things.  I know Node.nodeType used to be replaceable in a window - is Node itself replaceable in a content window?

Overall, this does make sense.  With the wide implementation of class info mirroring in DOM, where methods and properties are reflected into JS land for you, web pages shouldn't need Components.interfaces, and arguably shouldn't have access to it.

(It could be argued that Components itself shouldn't be exposed to content.  In chrome scope, I regularly check exception values against Components.results, which I don't think is available to content.  To my knowledge, nothing else under the Components umbrella is available to content.)
Comment 12 Reşat SABIQ (Reshat) 2011-04-16 22:18:21 PDT
Do i understand correctly that only interfaces used for native components are exposed via Components.interfaces? 
E.g., I used:
javascript:for each (i in Components.interfaces) { document.write(i); document.write("<br/>");}

I can see a native interface from an extension, but i cannot see any trace of a component (service) implemented in a js file. Am i missing something? Or does it just mean that there's a way to implement js components without having them exposed by Components.interfaces? If so, we might want to mention the availability of the work-around for extension developers using js-based components (in documentation, etc.).
Comment 13 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2011-04-18 08:06:23 PDT
no, any interface which has an xpt file is available in Components.interfaces.

Note that JS components don't necessarily have new interfaces: they could only implement existing interfaces, in which case you wouldn't be able to tell using only Components.interfaces whether they were installed.

interfaces != components
Comment 14 Reşat SABIQ (Reshat) 2011-04-18 19:03:01 PDT
(In reply to comment #13)
> no, any interface which has an xpt file is available in Components.interfaces.
I see. I only have experience with an interface for a native component (and compiling it into a typelib (xpt) file). 
I've looked at a few extensions, and among the ones i've looked at whenever there's an xpt file in an extension, there's also a native library (thus a native component). But it sounds like you are saying an extension without a native component can have an xpt as well. I'm totally cool with that, if that's the case, but is this likely to be common among js-based extensions using only js-based components? I'm just trying to get a sense of how severe an issue this is, in terms of a ratio of extensions and their users that it's likely to affect.

> Note that JS components don't necessarily have new interfaces: they could only
> implement existing interfaces, in which case you wouldn't be able to tell using
> only Components.interfaces whether they were installed.
This is probably a good news for a lot of extensions. This issue definitely still affects my native component, so i do want it fixed (ASAP :) ), but perhaps it doesn't affect the majority of extensions out there...
Comment 15 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2011-04-19 13:58:54 PDT
It's pretty uncommon for pure-JS extensions to need XPT files, because one of their primary purposes is to enable JS<->binary communication through xpconnect. And we're trying to discourage and probably eventually prevent people from using binary components in the future, moving everyone to JS-ctypes if they have the need for a binary interface.
Comment 16 arno renevier 2011-07-25 14:06:33 PDT
Created attachment 548281 [details] [diff] [review]
proposed patch

Here is a patch that removes access to Components.interfaces and Components.interfacesByID.

Basically, it removes interfaces and interfacesByID from list of properties to check in nsXPCComponents::CanGetProperty
I think that's enough to prevent access from content.
But then, it looks like there's no use for nsXPCComponents_Interfaces and  nsXPCComponents_InterfacesByID to implement nsISecurityCheckedComponent. Do I misunderstand here ?
Also, Components.results is currently not available from content, so I also removed it from access list in nsXPCComponents::CanGetProperty.
Comment 17 Reşat SABIQ (Reshat) 2011-07-25 19:46:17 PDT
(In reply to comment #15)
I would change the title of this bug to something like:
"Exposing Components.interfaces to untrusted content leaks information about some installed extensions (those using a .xpt file)"

Or maybe a bit more detailed parenthesized part: "(those using a .xpt file (compiled from an interface))".

P.S.
(In reply to comment #10)
Jokingly, Tor and EFF could give 10% of the reward each to me and Benjamin, because we have shown that absolute majority of the extensions and users are not affected by this bug (by ratio of extensions and users). ;)
P.P.S. I believe another way for the time being for an extension to avoid such exposure is to be implemented using a plugin instead of a native interface.
P.P.P.S. That said, i can't wait for this bug to be resolved. Thanks Arno.
Comment 18 :Ehsan Akhgari (away Aug 1-5) 2011-08-09 13:59:21 PDT
Pushed to inbound.
Comment 19 :Ehsan Akhgari (away Aug 1-5) 2011-08-09 15:18:24 PDT
I had to back this out from inbound because of mochitest failures.  See http://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=d9f175f9a2e4 for example.
Comment 20 Phil Ringnalda (:philor, back in August) 2011-08-09 15:51:15 PDT
s/mochitest/mochitest, reftest, crashtest, talos a11y/
Comment 21 arno renevier 2011-08-09 21:53:20 PDT
Sorry for the bustage, I had definitively not tested it well enough

I wasn't aware so many things were dependent on Components.interfaces. For some uses (especially, nsIDOMWindowUtils), there seem to be no easy alternative. Then, not exposing Component.interfaces looks much more difficult than I initially expected.
Comment 22 Ted Mielczarek [:ted.mielczarek] 2011-08-10 05:07:33 PDT
Note that in bug 462483 we're trying to remove all usage of XPCOM in Mochitest. I added a "window.SpecialPowers.DOMWindowUtils" at some point, does that continue to work with this patch? If so, then you may just need to wait until we finish that (long, ongoing) work.

You'll want to look into the reftest/crashtest bustage, though.
Comment 23 arno renevier 2011-08-11 05:02:03 PDT
(In reply to Ted Mielczarek [:ted, :luser] from comment #22)
> You'll want to look into the reftest/crashtest bustage, though.

There a 6 falling tests: 4 reftests, and 2 crashtests. There are xul tests which need to use methods using Components.interfaces.
May be they can go to chrome mochitests. Is there something like chrome-crashtest ?
Comment 24 Ted Mielczarek [:ted.mielczarek] 2011-08-11 05:55:36 PDT
We don't currently have anything to run reftests/crashtests with privileges. What exactly are these tests doing? Note that we don't actually support running XUL on the web anymore...
Comment 25 arno renevier 2011-08-11 06:05:16 PDT
For example, crashtest 434458-1.xul calls menupopup.popupBoxObject which is:
    this.boxObject.QueryInterface(Components.interfaces.nsIPopupBoxObject);


http://hg.mozilla.org/mozilla-central/file/10915aa17365/layout/xul/base/src/crashtests/434458-1.xul

http://hg.mozilla.org/mozilla-central/file/10915aa17365/toolkit/content/widgets/popup.xml#l18
Comment 26 Sean Newman 2011-09-04 12:21:15 PDT
I request to raise the importance of that bug.
That's a real leak of information and needs to be fixed asap.
Comment 27 Jesse Ruderman 2011-09-04 13:44:10 PDT
Moving those XUL crashtests to mochitest sounds reasonable.
Comment 28 Lea Verou 2011-09-16 10:07:08 PDT
If you "fix" this, there will be absolutely NO way to enumerate over child interfaces of HTMLElement (see this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=687042 ), which is especially useful for Firefox, since it implements many child methods on these interfaces, instead of just on Node or Element (see this: https://bugzilla.mozilla.org/show_bug.cgi?id=456151 ).

Please, if you change how Components works, at least fix one of the other bugs too. Otherwise the only solution for developers that want to do something that requires all the interfaces, would be to include a 0.5KB predefined list in their code.
Comment 29 Jesse Ruderman 2012-02-07 16:01:29 PST
If we want to fix both this and bug 693733, it might be easiest to remove Components from web pages entirely, rather than blocking access to its individual properties.
Comment 30 Johnny Stenback (:jst, jst@mozilla.com) 2012-02-07 16:04:19 PST
Agreed.
Comment 31 Camilo Viecco (:cviecco) 2012-02-10 09:46:33 PST
Solving this, requires first modification and cleanup of the test suite. There are two steps to do this.
Comment 32 Jesse Ruderman 2012-02-22 21:43:03 PST
I used my favorite method for estimating how much Gecko-specific code uses Components.interfaces:

https://www.google.com/search?q=site:userscripts.org%20%22components.interfaces%22%20-%22components.classes%22%20-gmIGreasemonkeyService

There were only a few hits, and they were all for scripts grabbing constants off of nsIDOMNodeFilter and nsIDOMXPathResult, which can be done using standard techniques (e.g. NodeFilter instead of Components.interfaces.nsIDOMNodeFilter).
Comment 33 Jesse Ruderman 2012-07-16 16:00:39 PDT
https://www.google.com/search?q=site:userscripts.org%20%22components.interfaces%22%20-%22components.classes%22%20-%22include%20chrome%22

There are also a few user scripts that sniff for gmIGreasemonkeyService in order to choose appropriate code paths for Firefox and Chrome.  http://stackoverflow.com/questions/1622145/how-can-i-mimic-greasemonkey-firefoxs-unsafewindow-functionality-in-chrome even has the gall to call this technique "feature sniffing".

Web sites won't sniff for gmIGreasemonkeyService for this reason, but they might sniff for gmIGreasemonkeyService in order to block Greasemonkey users from accessing the site.
Comment 34 Wladimir Palant 2012-07-17 04:52:20 PDT
GreaseMonkey scripts don't really run in web context, they are merely sandboxed. GreaseMonkey could easily expose Components.interfaces.gmIGreasemonkeyService to them (or at least a dummy with that name) just as it exposes its API.
Comment 35 Tom Schuster [:evilpie] 2013-04-06 07:43:45 PDT
Shouldn't this be fixed now after the landing of Bug 790732?
Comment 36 Andrew McCreight [:mccr8] 2013-04-06 07:55:09 PDT
bholley should know for sure, but it does seem like it.  There's some kind of shim for Components now, but I think that just exposes the same thing for everybody.
Comment 37 Bobby Holley (:bholley) (busy with Stylo) 2013-04-07 20:15:50 PDT
(In reply to Andrew McCreight [:mccr8] from comment #36)
> bholley should know for sure, but it does seem like it.  There's some kind
> of shim for Components now, but I think that just exposes the same thing for
> everybody.

Yeah, we only shim interfaces that expose DOM constants (see kInterfaceShimMap in nsDOMClassInfo.cpp), which is the same for everyone.

So this is fixed by that bug, but note that the behavior there is behind a pref that we may need to twiddle on branch. I'm not aware of any great way we have for tracking that kind of dependency.
Comment 38 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2013-04-08 06:18:29 PDT
The details are probably not important.

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