Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Reading local files using helper apps and XBL

RESOLVED FIXED

Status

()

Core
Security
RESOLVED FIXED
15 years ago
13 years ago

People

(Reporter: Mitchell Stoltz (not reading bugmail), Assigned: Mitchell Stoltz (not reading bugmail))

Tracking

Trunk
x86
Windows 2000
Points:
---
Bug Flags:
blocking1.4b +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 1 obsolete attachment)

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
(Assignee)

Comment 1

15 years ago
Created attachment 119464 [details]
localfile.html
(Assignee)

Comment 2

15 years ago
Created attachment 119465 [details]
localfile2.html
(Assignee)

Comment 3

15 years ago
Created attachment 119466 [details]
raw4.pl
(Assignee)

Comment 4

15 years ago
Created attachment 119467 [details]
raw4xbl.pl

Comment 5

15 years ago
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?
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

15 years ago
> 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

15 years ago
Note: I see no CheckLoadURI calls in nsXBLService.  This is very wrong....
>> 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

15 years ago
> 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...
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

15 years ago
Bug 60203 and its ten or so duplicates.
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.
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.
(Assignee)

Updated

15 years ago
Flags: blocking1.4b?
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

15 years ago
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).
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

15 years ago
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?
(Assignee)

Comment 19

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

Updated

14 years ago
Flags: blocking1.4b? → blocking1.4b+
(Assignee)

Comment 20

14 years ago
Created attachment 121742 [details] [diff] [review]
Patch - add CheckLoadURI call to XBL loader

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

Comment 21

14 years ago
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.
Attachment #121742 - Flags: superreview?(heikki)
Attachment #121742 - Flags: review?(bryner)
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
Attachment #121742 - Flags: superreview?(heikki) → superreview+
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.
Attachment #121742 - Flags: review?(bryner) → review-
(Assignee)

Comment 24

14 years ago
Created attachment 122158 [details] [diff] [review]
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.
Attachment #121742 - Attachment is obsolete: true
(Assignee)

Updated

14 years ago
Attachment #122158 - Flags: superreview?(heikki)
Attachment #122158 - Flags: review?(bryner)
Attachment #122158 - Flags: superreview?(heikki) → superreview+
Comment on attachment 122158 [details] [diff] [review]
Better patch - no perf hit, moved check at bryner's request

r=bryner
Attachment #122158 - Flags: review?(bryner) → review+
(Assignee)

Comment 26

14 years ago
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.
Attachment #122158 - Flags: approval1.4b?

Comment 27

14 years ago
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.
Attachment #122158 - Flags: approval1.4b? → approval1.4b+
(Assignee)

Comment 28

14 years ago
Fix checked in.
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Removing security-sensitive flag for bugs on the known-vulnerabilities list
Group: security
You need to log in before you can comment on or make changes to this bug.