Closed
Bug 200691
Opened 22 years ago
Closed 22 years ago
Reading local files using helper apps and XBL
Categories
(Core :: Security, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: security-bugs, Assigned: security-bugs)
References
Details
Attachments
(5 files, 1 obsolete file)
(deleted),
text/html
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
application/octet-stream
|
Details | |
(deleted),
application/octet-stream
|
Details | |
(deleted),
patch
|
bryner
:
review+
hjtoi-bugzilla
:
superreview+
asa
:
approval1.4b+
|
Details | Diff | Splinter Review |
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•22 years ago
|
||
Assignee | ||
Comment 2•22 years ago
|
||
Assignee | ||
Comment 3•22 years ago
|
||
Assignee | ||
Comment 4•22 years ago
|
||
Comment 5•22 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?
Comment 6•22 years ago
|
||
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•22 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•22 years ago
|
||
Note: I see no CheckLoadURI calls in nsXBLService. This is very wrong....
Comment 9•22 years ago
|
||
>> 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•22 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...
Comment 11•22 years ago
|
||
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•22 years ago
|
||
Bug 60203 and its ten or so duplicates.
Comment 13•22 years ago
|
||
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•22 years ago
|
||
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•22 years ago
|
Flags: blocking1.4b?
Comment 15•22 years ago
|
||
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•22 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).
Comment 17•22 years ago
|
||
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•22 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•22 years ago
|
||
*** Bug 195317 has been marked as a duplicate of this bug. ***
Updated•22 years ago
|
Flags: blocking1.4b? → blocking1.4b+
Assignee | ||
Comment 20•22 years ago
|
||
I think this will do the trick - need a good testcase.
Assignee | ||
Comment 21•22 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 23•22 years ago
|
||
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•22 years ago
|
||
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•22 years ago
|
Attachment #122158 -
Flags: superreview?(heikki)
Attachment #122158 -
Flags: review?(bryner)
Updated•22 years ago
|
Attachment #122158 -
Flags: superreview?(heikki) → superreview+
Comment 25•22 years ago
|
||
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•22 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•22 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•22 years ago
|
||
Fix checked in.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 29•20 years ago
|
||
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.
Description
•