Closed Bug 200691 Opened 22 years ago Closed 22 years ago

Reading local files using helper apps and XBL

Categories

(Core :: Security, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: security-bugs, Assigned: security-bugs)

References

Details

Attachments

(5 files, 1 obsolete file)

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
Attached file localfile.html (deleted) —
Attached file localfile2.html (deleted) —
Attached file raw4.pl (deleted) —
Attached file raw4xbl.pl (deleted) —
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.
> 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? :(
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.
> 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
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.
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.
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.
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?
*** Bug 195317 has been marked as a duplicate of this bug. ***
Flags: blocking1.4b? → blocking1.4b+
Attached patch Patch - add CheckLoadURI call to XBL loader (obsolete) (deleted) — Splinter Review
I think this will do the trick - need a good testcase.
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-
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
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+
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 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+
Fix checked in.
Status: NEW → RESOLVED
Closed: 22 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.

Attachment

General

Created:
Updated:
Size: