200691 - Reading local files using helper apps and XBL

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[Mitchell Stoltz (not reading bugmail)]

It is possible to read local files with the help of external applications
and a bug in xbl.
Works at least on linux, probably with small modifications on other OSes.
The problem is that when an external document is opened with external
application, a file with known filename is created in /tmp.
So locxbl.xbl is created in /tmp.
Then xxxx.html is created in /tmp.
(both with content-type video/mpeg).
localfile2.html uses locxbl.xbl from the local filesystem.
Then locxbl.xbl opens xxxx.html which reads /etc/groups.

How to reproduce on localhost (may work on another hosts, but localhost
should be changed and the perl scripts should be started on the other
host).
1. start raw4xbl.pl on localhost
2. start raw4.pl on localhost
3. open localfile.html from the web.
4. In localfile.html first click on the "1. ..." link then chose "open
...using application" if prompted.

5. In localfile.html first click on the "2. ..." link then chose "open ..."
if prompted.
6. In localfile.html click on 3. and see the contents of /etc/group read.

In case the user has chosen to always open certain filetypes (which seems
the default after first opening) the exploit works automatically.
Shall check on windows - media files seems more suitable for this attack.

On windoze /tmp should be replaced with something else and change of the
content may be needed. In case netscape registers winamp or other player
they may be used.

All needed files are attached.

Georgi

Comment 1

[Mitchell Stoltz (not reading bugmail)]

Attachment: localfile.html

Comment 2

[Mitchell Stoltz (not reading bugmail)]

Attachment: localfile2.html

Comment 3

[Mitchell Stoltz (not reading bugmail)]

Attachment: raw4.pl

Comment 4

[Mitchell Stoltz (not reading bugmail)]

Attachment: raw4xbl.pl

Comment 5

[Boris Zbarsky [:bzbarsky]]

I'm a little confused... is the issue that remote content can load XBL from a
file:// url, basically?  If not, what is the problem?

Comment 6

[georgi - hopefully not receiving bugspam]

Boris,

There are several small problems which make one big problem.
Here are they:
1. It is possible to do XBL binding to local XBL via style="-moz-binding:
url('file:///tmp/locxbl.xbl#root')">
2. XBL loaded from file URL allows loading local files via location.replace()
3. External applications create files with known names in a known location which
allows creating local xbl and html.

This exploit requires minimal user interaction, but if some file types are
always opened, then the risk is greatly increased.

Comment 7

[Boris Zbarsky [:bzbarsky]]

> 1. It is possible to do XBL binding to local XBL via style="-moz-binding:
> url('file:///tmp/locxbl.xbl#root')">

This is not a little problem.  This is a major security hole.  You should not be
able to do that if you're not a local file or chrome yourself.

> 2. XBL loaded from file URL allows loading local files via location.replace()

Nothing wrong with that, imo.... Local HTML allows that too....

> 3. External applications create files with known names in a known location 
> which allows creating local xbl and html.

Yeah...  we used to not do this, but there was a lot of noise about it, because
people want to be able to get at the files after the helper app is done with
them...  Any ideas?  :(

Comment 8

[Boris Zbarsky [:bzbarsky]]

Note: I see no CheckLoadURI calls in nsXBLService.  This is very wrong....

Comment 9

[georgi - hopefully not receiving bugspam]

>> 2. XBL loaded from file URL allows loading local files via location.replace()

>Nothing wrong with that, imo.... Local HTML allows that too....

This wrong IMHO. The XBL is loaded from html/xml via <style> and not directly.
It is not like a standalone document, it seems more like <script> or
xul-overlay. XBL is not the same as local HTML IMHO.
Not an expert in XBL though, so may be wrong. 

>> 3. External applications create files with known names in a known location 
>> which allows creating local xbl and html.

>Yeah...  we used to not do this, but there was a lot of noise about it, because
>people want to be able to get at the files after the helper app is done with
>them...  Any ideas?  :(

I vote for removing this dangerous stuff. This may open path to more exploits.
People can always shift click on the link and do save as.

Comment 10

[Boris Zbarsky [:bzbarsky]]

> it seems more like <script> or xul-overlay

Yes, but a local script or overlay takes the permissions of the URL it was
loaded from as well...For example, a <script> loaded from file:/// can
XMLHttpRequest from anywhere (no SameOrigin check).

As for the naming issue, I totally agree from a security standpoint.  But in
practice, that was pretty much unacceptable (see the bugs).  Which is part of
why we have CheckLoadURI...

Comment 11

[Brendan Eich [:brendan]]

Boris, what bugs should I see to learn why people demand helper app files with
well-known names?

1 and 3 seem like problems, 1 of course.  2 is questionable.  We don't allow
non-file: pages to load file: scripts, IIRC (MozillaClassic disallowed this, in
any event), but we've always allowed local pages to load local scripts via
script src=file:... or a relative URL.  How is XBL different from a security
standpoint from HTML with inline scripts?

/be

Comment 12

[Boris Zbarsky [:bzbarsky]]

Bug 60203 and its ten or so duplicates.

Comment 13

[georgi - hopefully not receiving bugspam]

Predictable filenames caused a lot of problems to internet exploder in the past
at least.
To keep users happy I suggest the following.
1. Place the opened file under the "salty" directory
and
2. Add some salt to it, so blondeporno.mpg becomes blondeporno{RANDOMSALT}.mpg
Users can always remove the obfuscated part if they wish.

Comment 14

[georgi - hopefully not receiving bugspam]

A small rant:
Bug 60203 does not justify leaving known filenames IMHO.
If users want known filename, they should chose Save As.
Users may want whatever features, but it is mozilla's developers responsibility
to ship a secure browser.

Comment 15

[georgi - hopefully not receiving bugspam]

IMHO a quick fix will be to remove the predictable filenames - this will fix
also unknown attacks.
The XBL problem should alse be taken care of.

Comment 16

[Boris Zbarsky [:bzbarsky]]

An even quicker fix is to disable helper applications entirely.  Which doesn't
mean we should do that.

Fixing the core XBL problem should not be very hard -- just make a call to the
security manager checking whether loading XBL from the target URL is allowed by
the security policies.

That said, if someone provides a patch to do salting per comment 13, I'm willing
to review (but I have no time to work on one).

Comment 17

[georgi - hopefully not receiving bugspam]

Boris,

Fixing the XBL problem with stop *this* exploit.
But if someone manages to find a way to render local files via XUL/XML/XBL/etc,
this gives him instant access to reading local files via the same method.
I have the impression that helper files used to be salty, so probably it won't
be hard to implement comment 13 for example.

Comment 18

[Jesse Ruderman]

Why do the permissions of scripts in XBL depend on the location of the XBL
rather than the location of the page they're included in?

Comment 19

[Mitchell Stoltz (not reading bugmail)]

*** Bug 195317 has been marked as a duplicate of this bug. ***

Comment 20

[Mitchell Stoltz (not reading bugmail)]

Attachment: Patch - add CheckLoadURI call to XBL loader

I think this will do the trick - need a good testcase.

Comment 21

[Mitchell Stoltz (not reading bugmail)]

Comment on attachment 121742 [details] [diff] [review]
Patch - add CheckLoadURI call to XBL loader

OK, the patch seems to work, and no regressions I can find.

Comment 22

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Comment on attachment 121742 [details] [diff] [review]
Patch - add CheckLoadURI call to XBL loader

Have you checked perf numbers? If there is a noticeable hit you may need to
cache creation of secMan, and possibly reusing bindingURI object to avoid some
allocations.

Nit: I would suggest moving creation of secMan forward to where it is actually
used.

Provided there is no noticeable perf hit then sr=heikki

Comment 23

[Brian Ryner (not reading)]

Comment on attachment 121742 [details] [diff] [review]
Patch - add CheckLoadURI call to XBL loader

Could we move this check down so that it's just above the call to GetBinding()
? (below the call to nsBindingManager::GetBinding and the block of code after
it).

That shoud save us from doing the security check if the binding has already
been installed on the element.

Comment 24

[Mitchell Stoltz (not reading bugmail)]

Attachment: Better patch - no perf hit, moved check at bryner's request

The other patch increased Txul by 9% - added a check for chrome URIs and
there's no detectable perf hit. I also moved the check at bryner's request.

Comment 25

[Brian Ryner (not reading)]

Comment on attachment 122158 [details] [diff] [review]
Better patch - no perf hit, moved check at bryner's request

r=bryner

Comment 26

[Mitchell Stoltz (not reading bugmail)]

Comment on attachment 122158 [details] [diff] [review]
Better patch - no perf hit, moved check at bryner's request

Requesting approval for 1.4b. This patch prevents remote XUL files from using
XBL files from file:// URLs (chrome: is still allowed). Since chrome XUL files
are excluded from the check, this only affects remote XUL files, so the risk is
low.

Comment 27

[Asa Dotzler [:asa]]

Comment on attachment 122158 [details] [diff] [review]
Better patch - no perf hit, moved check at bryner's request

a=asa (on behalf of drivers) for checkin to 1.4b.

Comment 28

[Mitchell Stoltz (not reading bugmail)]

Fix checked in.

Comment 29

[Daniel Veditz [:dveditz]]

Removing security-sensitive flag for bugs on the known-vulnerabilities list