Page can obtain path to Mozilla installation or possibly profile by examining JavaScript exceptions
Categories
(Core :: Security, defect, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox75 | --- | fixed |
People
(Reporter: bugzilla, Assigned: bzbarsky)
References
(Blocks 1 open bug)
Details
(Keywords: privacy, sec-want, Whiteboard: [sg:want] stepping-stone [fingerprinting][fp-triaged][adv-main75-])
Attachments
(3 files)
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; cs-CZ; rv:1.7.3) Gecko/20040910 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; cs-CZ; rv:1.7.3) Gecko/20040910 I am not sure if this should be considered as security bug. If you don't think so, open it. Page which call window.sidebar.addSearchEngine() with bad agruments (bad suffix, protocol) can catch the exception of the Mozilla code and from it obtain path to the Mozilla installlation. Problem is in the nsSidebar.js (in Mozilla and Firefox too) containing line: this.promptService.alert(null, "Failed to add the search engine."); there is missing the 3th alert argument (text of the dialog), which raises expception e.g.: [Exception... "Not enough arguments [nsIPromptService.alert]" nsresult: "0x80570001 (NS_ERROR_XPC_NOT_ENOUGH_ARGS)" location: "JS frame :: file:///C:/Program%20Files/mozilla.org/Mozilla/components/nsSidebar.js :: anonymous :: line 266" data: no] HTML page can catch this exception and obtain the path of the problematic file (i.e. the path of Mozilla installation). Revealing the path to the nsSidebar.js is not probably critical problem, but what if the exception raises in some extension in the user profile and the page can catch path to the user profile (and the user login)? So problem is propably more general. Should be the HTML page able to catch exceptions from the Mozilla code? And if should, should it obtain a full text of the expections (including file path)? Reproducible: Always Steps to Reproduce:
Reporter | ||
Comment 1•20 years ago
|
||
Comment 2•20 years ago
|
||
Note that there are other ways that sidebar can throw exceptions, e.g. sidebar.addPanel(null, null, null);
Assignee | ||
Comment 3•20 years ago
|
||
Perhaps that should be a chrome URI, not a file one?
Comment 4•20 years ago
|
||
nsSidebar.js doesn't live in chrome, it's a component.
Comment 5•20 years ago
|
||
I'm not too worried about revealing the installation directory (it's generally in an easily guessed default location), but Met is absolutely right that this could potentially reveal the profile location. Would require: - user installs Firefox extension (profile-based) - extension uses a javascript component (chrome js exceptions would just reveal that the extension exists) - extension exposes their component to web content somehow -- probably rare Then the attacker would also have to know of a local-file-stealing exploit for the profile location to be useful. In limited situations just knowing the OS-login name might be useful to an attacker and that would usually be revealed (unless the user chose a non-default profile location, and that's not generally possible in Firefox without secret command-line args). One "fix" for this bug would be to prevent the sidebar component from throwing exceptions into user scripts. That seems like the friendly thing to do in any case for sidebar, but that doesn't fix the potential for this to occur in some popular but badly coded extension. Rather we need to fix this at the JS exception level somehow. Blue skying: if the exception is going to put a file: url in there just list the filename, but strip the rest of the path. That's a little severe and hurts people trying to develop apps on their local disk. Could perhaps check whether the top level script is priveged or not, or what scheme it came from. Or just strip paths by default and let developers flip a pref to get full paths like the plugin.expose_full_path pref. Nominating for 1.0 just to get consideration, but without knowing of a specific profile-installed extension that can be induced to throw an exception into web content I don't think we need to take this kind of change so late. Good 1.1/1.8 fodder.
Comment 6•20 years ago
|
||
gonna work on this for the next release
Updated•19 years ago
|
Updated•19 years ago
|
Updated•19 years ago
|
Comment 7•18 years ago
|
||
Also true for FF 1.5.0.1RC1 on Mac OS X
Assignee | ||
Updated•18 years ago
|
Comment 8•18 years ago
|
||
I marked this as blocked by bug 268370 because it fixes the specific case mentioned here, but re-reading this bug, I see that's it's looking for a general solution to the problem, so I'll revert that change.
Comment 9•18 years ago
|
||
Information about the testcase was posted to Bugtraq list on May 22th listing it as PoC: http://www.securityfocus.com/archive/1/434696/30/30/threaded Original time stamp at posting mwntioned is May 21 2006 because of moderating process of Symantec.
Comment 10•18 years ago
|
||
also now posted as secunia http://secunia.com/advisories/20256/ (suite) and http://secunia.com/advisories/20244/ firefox
Comment 11•18 years ago
|
||
I've posted a 1.8.0-compatible branch patch for the one known case of this problem in bug 268370. I've posted a better patch for 1.8 and trunk in bug 338989.
Comment 12•18 years ago
|
||
Also true for Firefox 2.0a2 (Bon Echo) on Fedora Core 4, kernel 2.6.14-1.1653_FC4, j2sdk-1.5.0_05-1_FC4.
Assignee | ||
Comment 13•18 years ago
|
||
Could we have our exception objects implement nsISecurityCheckedComponent and do a CanAccess check from the caller principal to the filename URI? For DOMException this could be tough because it has classinfo claiming it's a DOM object and therefore would short-circuit the nsISecurityCheckedComponent check, but maybe we can change said classinfo?
Updated•18 years ago
|
Updated•18 years ago
|
Comment 14•18 years ago
|
||
(In reply to comment #11) > I've posted a 1.8.0-compatible branch patch for the one known case of this > problem in bug 268370. That's a perfectly fine fix that we want, but we should not claim it fixes this bug (I don't think it even fixes neil's comment 2 testcase).
Comment 15•18 years ago
|
||
(In reply to comment #14) > (In reply to comment #11) > > I've posted a 1.8.0-compatible branch patch for the one known case of this > > problem in bug 268370. > > That's a perfectly fine fix that we want, but we should not claim it fixes > this bug (I don't think it even fixes neil's comment 2 testcase). You're right, I should have said "for one known case of this" instead of "for the one known case of this". It does not fix the case from comment 2. I'll post a patch that does, because it's a trivial fix, but getting a better general solution to this bug would obviously be better than trying to play whack-a-mole with all the places this can happen.
Comment 16•18 years ago
|
||
With the only known instance fixed (bug 268370) we can push the more general solution out to a more appropriate place like trunk or 1.8 branch.
Updated•18 years ago
|
Comment 17•18 years ago
|
||
Not going to block on this for release of Firefox 2, but we'd take a baked patch if one comes to us (nominate for approval1.8.1)
Updated•17 years ago
|
Updated•16 years ago
|
Comment 18•16 years ago
|
||
The following test case is not able to catch anything on trunk. try { window.sidebar.addPanel(null,null,null); } catch(e) { alert(e); } And the following is logged to the error console: Error: Invalid argument passed to window.sidebar.addPanel: Unsupported panel URL. Source File: file:///C:/Program%20Files/Mozilla%20Firefox%203%20Beta%203/components/nsSidebar.js Line: 47 Is this bug still valid on trunk?
Comment 19•16 years ago
|
||
(In reply to comment #18) > Is this bug still valid on trunk? Yes, see comment 16. There may be other unknown errors that will expose similar problems, so we need a general solution, rather than relying on being able to identify all the potential problem areas.
Updated•16 years ago
|
Updated•16 years ago
|
Updated•15 years ago
|
Updated•11 years ago
|
Updated•6 years ago
|
Assignee | ||
Comment 21•6 years ago
|
||
I wanted to give a quick update on the current state of this, which has changed a lot since comment 19. Once bug 888600 lands (matter of days, I hope), all web-exposed things will be either WebIDL or some sort of direct injection via exportFunction. When calling through WebIDL, if a JS exception is thrown as part of the _implementation_, one of the following things happens: * If this is JS-implemented WebIDL, then CallbackObject::CallSetup::ShouldRethrowException does a security check and will not expose exceptions thrown by chrome to webpages. They will be reported to the console and a sanitized exception thrown to the webpage. We have tests for this behavior. * If this is C++-implemented WebIDL that calls a JS XPCOM component that throws, we will end up in dom::ThrowExceptionValueIfSafe and again do a security check to make sure it's OK to expose the exception. See bug 1295322 for where we changed that. So I believe we have now put in place the general solution this bug was about, for WebIDL. For the direct-injection case... I'm not sure what the situation is.
Comment 22•5 years ago
|
||
Can we close this bug according to comment 21?
Comment 23•5 years ago
|
||
That's hard to say since the situation of the direct-injection case is still unclear. I think we should also make sure that the installation path won't be leaked through direct-injection codes before we close this.
Updated•5 years ago
|
Updated•5 years ago
|
Comment 24•5 years ago
|
||
We talked to :baku about this and he thinks we need to sanitize the direct injection case. We can look for an Exception in the JSContext, take it out, sanitize it if it's there, and then put it back. We can reuse the same type of sanitization we already have. But first we need to write a test to show it failing.
Comment 25•5 years ago
|
||
Updated•4 years ago
|
Assignee | ||
Comment 26•4 years ago
|
||
OK, I tried editing the testcase to from comment 25 make sense: to make the sandbox less privileged than the exported function, and to examine the state the sandbox sees, not the state the privileged caller sees.
As things stand, the sandbox ends up with a direct reference (via CCWs, of course) to the privileged exception, and attempting to get properties off it results in security errors.
Is that good enough, or do we still want to do some sort of sanitizing behavior to prevent those security errors?
Updated•4 years ago
|
Assignee | ||
Comment 27•4 years ago
|
||
Anyway, I posted a proposed fix that does the sanitizing at https://phabricator.services.mozilla.com/D35996
Assignee | ||
Comment 28•4 years ago
|
||
Updated•4 years ago
|
Comment 29•4 years ago
|
||
(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #26)
Is that good enough, or do we still want to do some sort of sanitizing behavior to prevent those security errors?
It looks good to me.
Tom, can you take a look?
Comment 30•4 years ago
|
||
(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #26)
OK, I tried editing the testcase to from comment 25 make sense: to make the sandbox less privileged than the exported function, and to examine the state the sandbox sees, not the state the privileged caller sees.
As things stand, the sandbox ends up with a direct reference (via CCWs, of course) to the privileged exception, and attempting to get properties off it results in security errors.
Is that good enough, or do we still want to do some sort of sanitizing behavior to prevent those security errors?
Truthfully I don't have a solid grasp of everything here, but it seems like it would be okay to me. Could one probe to see if a property exists (returns a security error) vs doesn't and infer something about the user's machine from it? That seems unlikely - they would probably just be able to infer something about the exception thrown which isn't concerning, because they caused it to be thrown.
Comment 31•4 years ago
|
||
Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/942a92874526 part 1. Add a way to ask for DOMException to be exposed in sandboxes. r=bholley
Comment 32•4 years ago
|
||
Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/039e8fd2928d part 2. Sanitize exceptions from exported functions. r=xeonchen,bholley
Comment 33•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/942a92874526
https://hg.mozilla.org/mozilla-central/rev/039e8fd2928d
Updated•4 years ago
|
Description
•