Closed Bug 429070 Opened 16 years ago Closed 11 years ago

exposing Components.interfaces to untrusted content leaks information about installed extensions

Categories

(Core :: XPConnect, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: HeroreV, Assigned: cviecco)

References

(Blocks 1 open bug, )

Details

(Keywords: addon-compat, privacy, sec-low, Whiteboard: [sg:low][tor])

Attachments

(1 file)

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.
Keywords: privacy
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.
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... 
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.
What can I do to confirm this one?
Moving to correct component (from what I can tell) and confirming.
Blocks: abp
Status: UNCONFIRMED → NEW
Component: DOM → XPCOM
Ever confirmed: true
QA Contact: general → xpcom
Why does remote XUL need access to Components.interfaces? for accessibility stuff?
Component: XPCOM → XPConnect
QA Contact: xpcom → xpconnect
To implement XPCOM components (QueryInterface). At the very least it was necessary to implement nsITreeView in remote XUL but now that is forbidden anyway.
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).
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.
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
Whiteboard: [sg:low]
(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.)
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.).
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
(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...
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.
Attached patch proposed patchSplinter Review
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.
Attachment #548281 - Flags: review?(mrbkap)
(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.
Attachment #548281 - Flags: review?(mrbkap) → review+
Keywords: checkin-needed
Pushed to inbound.
Keywords: checkin-needed
I had to back this out from inbound because of mochitest failures.  See http://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=d9f175f9a2e4 for example.
s/mochitest/mochitest, reftest, crashtest, talos a11y/
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.
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.
Depends on: 462483
(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 ?
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...
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
I request to raise the importance of that bug.
That's a real leak of information and needs to be fixed asap.
Keywords: addon-compat
Moving those XUL crashtests to mochitest sounds reasonable.
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.
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.
Agreed.
Depends on: 726053
Assignee: nobody → cviecco
Solving this, requires first modification and cleanup of the test suite. There are two steps to do this.
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).
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.
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.
Depends on: 790732
Depends on: 808457
Shouldn't this be fixed now after the landing of Bug 790732?
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.
Flags: needinfo?(bobbyholley+bmo)
(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.
Flags: needinfo?(bobbyholley+bmo)
The details are probably not important.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Whiteboard: [sg:low] → [sg:low][tor]
Blocks: meta_tor
You need to log in before you can comment on or make changes to this bug.