Closed
Bug 429070
Opened 17 years ago
Closed 12 years ago
exposing Components.interfaces to untrusted content leaks information about installed extensions
Categories
(Core :: XPConnect, defect)
Core
XPConnect
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)
(deleted),
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
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•17 years ago
|
||
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•17 years ago
|
||
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•17 years ago
|
||
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•16 years ago
|
||
What can I do to confirm this one?
Comment 5•16 years ago
|
||
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
Comment 6•16 years ago
|
||
Why does remote XUL need access to Components.interfaces? for accessibility stuff?
Component: XPCOM → XPConnect
QA Contact: xpcom → xpconnect
Comment 7•16 years ago
|
||
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•14 years ago
|
||
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•14 years ago
|
||
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•14 years ago
|
||
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
Updated•14 years ago
|
Whiteboard: [sg:low]
Comment 11•14 years ago
|
||
(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•14 years ago
|
||
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•14 years ago
|
||
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•14 years ago
|
||
(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•14 years ago
|
||
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•13 years ago
|
||
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)
Comment 17•13 years ago
|
||
(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.
Updated•13 years ago
|
Attachment #548281 -
Flags: review?(mrbkap) → review+
Updated•13 years ago
|
Keywords: checkin-needed
Comment 19•13 years ago
|
||
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•13 years ago
|
||
s/mochitest/mochitest, reftest, crashtest, talos a11y/
Comment 21•13 years ago
|
||
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•13 years ago
|
||
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•13 years ago
|
||
(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•13 years ago
|
||
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•13 years ago
|
||
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•13 years ago
|
||
I request to raise the importance of that bug.
That's a real leak of information and needs to be fixed asap.
Updated•13 years ago
|
Keywords: addon-compat
Comment 27•13 years ago
|
||
Moving those XUL crashtests to mochitest sounds reasonable.
Comment 28•13 years ago
|
||
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•13 years ago
|
||
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•13 years ago
|
||
Agreed.
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → cviecco
Assignee | ||
Comment 31•13 years ago
|
||
Solving this, requires first modification and cleanup of the test suite. There are two steps to do this.
Comment 32•13 years ago
|
||
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•12 years ago
|
||
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•12 years ago
|
||
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•12 years ago
|
||
Shouldn't this be fixed now after the landing of Bug 790732?
Comment 36•12 years ago
|
||
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)
Comment 37•12 years ago
|
||
(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)
Comment 38•12 years ago
|
||
The details are probably not important.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
Target Milestone: --- → mozilla22
Updated•11 years ago
|
Whiteboard: [sg:low] → [sg:low][tor]
You need to log in
before you can comment on or make changes to this bug.
Description
•