267645 - Page can obtain path to Mozilla installation or possibly profile by examining JavaScript exceptions

Status: RESOLVED FIXED

(Core :: Security, defect, P3)

Description

[Met - Martin Hassman]

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:

Comment 1

[Met - Martin Hassman]

Attachment: Testcase

Comment 2

[[e-mail]]

Note that there are other ways that sidebar can throw exceptions, e.g.
sidebar.addPanel(null, null, null);

Comment 3

[Boris Zbarsky [:bzbarsky]]

Perhaps that should be a chrome URI, not a file one?

Comment 4

[[e-mail]]

nsSidebar.js doesn't live in chrome, it's a component.

Comment 5

[Daniel Veditz [:dveditz]]

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

[Asa Dotzler [:asa]]

gonna work on this for the next release

Comment 7

[Bob Clary [:bc] (inactive)]

Also true for FF 1.5.0.1RC1 on Mac OS X

Comment 8

[:Gavin Sharp [email: [e-mail]]]

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

[Juha-Matti Laurio]

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

[chris hofmann]

also now posted as secunia 
http://secunia.com/advisories/20256/ (suite) and http://secunia.com/advisories/20244/ firefox

Comment 11

[:Gavin Sharp [email: [e-mail]]]

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

[Cléoson Barreto]

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.

Comment 13

[Boris Zbarsky [:bzbarsky]]

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?

Comment 14

[Daniel Veditz [:dveditz]]

(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

[:Gavin Sharp [email: [e-mail]]]

(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

[Daniel Veditz [:dveditz]]

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.

Comment 17

[Mike Beltzner [:beltzner, not reading bugmail]]

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)

Comment 18

[(no longer active)]

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

[:Gavin Sharp [email: [e-mail]]]

(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.

Comment 20

[Tom Ritter [:tjr]]

Marking as P2 to investigate for fingerprinting

Comment 21

[Boris Zbarsky [:bzbarsky]]

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

[Ethan Tseng [:ethan]]

Can we close this bug according to comment 21?

Comment 23

[Tim Huang[:timhuang]]

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.

Comment 24

[Tom Ritter [:tjr]]

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

[Gary Chen [:xeonchen]]

Attachment: Bug 267645 part 2. Sanitize exceptions from exported functions.

Comment 26

[Boris Zbarsky [:bzbarsky]]

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?

Comment 27

[Boris Zbarsky [:bzbarsky]]

Anyway, I posted a proposed fix that does the sanitizing at https://phabricator.services.mozilla.com/D35996

Comment 28

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 267645 part 1. Add a way to ask for DOMException to be exposed in sandboxes.

Comment 29

[Gary Chen [:xeonchen]]

(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

[Tom Ritter [:tjr]]

(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

[Pulsebot]

Pushed by [e-mail]:
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

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/039e8fd2928d
part 2.  Sanitize exceptions from exported functions. r=xeonchen,bholley

Comment 33

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/942a92874526
https://hg.mozilla.org/mozilla-central/rev/039e8fd2928d