Last Comment Bug 337744 - resource:// can load any file on the disk
: resource:// can load any file on the disk
Status: RESOLVED FIXED
[sg:low dos]
: fixed1.8.0.8, fixed1.8.1
Product: Core
Classification: Components
Component: Networking: File (show other bugs)
: Trunk
: x86 Linux
: P1 normal (vote)
: mozilla1.8.1
Assigned To: Benjamin Smedberg [:bsmedberg]
:
Mentors:
resource:////tmp
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-05-12 11:30 PDT by Boris Zbarsky [:bz]
Modified: 2006-11-10 12:15 PST (History)
13 users (show)
benjamin: blocking1.9a1+
benjamin: blocking1.8.1+
timr: blocking1.8.0.4-
dveditz: blocking1.8.0.7-
dveditz: blocking1.8.0.8+
benjamin: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Check for colon, rev. 1 (2.51 KB, patch)
2006-07-11 11:34 PDT, Benjamin Smedberg [:bsmedberg]
darin.moz: review-
Details | Diff | Review
Check for colon, rev. 2 (1.49 KB, patch)
2006-07-11 11:48 PDT, Benjamin Smedberg [:bsmedberg]
darin.moz: review+
dveditz: approval1.8.0.7+
mtschrep: approval1.8.1+
Details | Diff | Review
Check for site-absolute path, rev. 1 (1.45 KB, patch)
2006-09-08 08:30 PDT, Benjamin Smedberg [:bsmedberg]
darin.moz: review+
dveditz: approval1.8.0.8+
mbeltzner: approval1.8.1+
Details | Diff | Review
Unit test, rev. 1 (670 bytes, patch)
2006-09-11 11:22 PDT, Benjamin Smedberg [:bsmedberg]
darin.moz: review+
Details | Diff | Review

Description Boris Zbarsky [:bz] 2006-05-12 11:30:50 PDT
STEPS TO REPRODUCE:  On a Unix system, type resource:////home in the URL bar.  Or  resource://// for that matter.

bsmedberg says this is not by-design.
Comment 1 Benjamin Smedberg [:bsmedberg] 2006-05-12 11:43:06 PDT
Need to add some kind of "is relative and doesn't have .." check here:
http://lxr.mozilla.org/mozilla/source/netwerk/protocol/res/src/nsResProtocolHandler.cpp#330
Comment 2 Benjamin Smedberg [:bsmedberg] 2006-05-12 11:50:24 PDT
Chrome registry does the same sort of relative lookup, and has these checks:

http://lxr.mozilla.org/mozilla/source/chrome/src/nsChromeRegistry.cpp#623 (prevents the relative part from beginning with a slash).

http://lxr.mozilla.org/mozilla/source/chrome/src/nsChromeRegistry.cpp#678
(prevents relative ..links)

And this leads me to a related bug, chrome://global/content/file:///Users resolved! This is serious, and IMO worth doing a quick 1505 or holding 1504 for.
Comment 3 Christian :Biesinger (don't email me, ping me on IRC) 2006-05-12 12:25:29 PDT
does it handle escaped dots as well?
Comment 4 Darin Fisher 2006-05-12 13:35:56 PDT
I'm wondering how this can be abused from a security point of view.  I can see how it would allow someone to load images off the users filesystem, but otherwise?  What else can be done?
Comment 5 Boris Zbarsky [:bz] 2006-05-12 13:40:14 PDT
  <img src="resource:////dev/stdin">

will effectively hang the browser on a typical Linux install.  There are similar file:// URIs that do major damage on Win9x (like crash the OS).  Loading a file as a stylesheet that you can then read via the CSSOM allows getting data off the user's hard drive and into the page's JS, insofar as that data looks like CSS.

Basically, all the reasons we don't allow images, scripts, etc to link to file://.
Comment 6 Benjamin Smedberg [:bsmedberg] 2006-05-14 11:58:00 PDT
resource:///http://www.google.com/ also works (though I can't think of a way to exploit this since you can load scripts/styles from the web directly).

chrome://global/content/http://www.google.com/ does *not* work, because of the channel-unwinding checks at http://lxr.mozilla.org/mozilla/source/chrome/src/nsChromeProtocolHandler.cpp#563

But if remote content were loaded from a JAR file it would work, e.g.:

chrome://global/content/jar:http://www.example.com/myjar.zip!/somefile
Comment 7 Daniel Veditz [:dveditz] 2006-05-15 12:27:14 PDT
Filed bug 338037 on the chrome: url issue in comment 6 and 2. Let's stick to the resource: problem here.
Comment 8 Tim Riley [:timr] 2006-05-17 09:12:27 PDT
dveditz and I discussed this and we feel it is not  critical for 1.8.0.4.  Particularly since we forked off the chrome: URL issue to another bug.  We'll try to pick it up for 1.8.0.5.  But we need a patch baked on the trunk.
Comment 9 Mike Schroepfer 2006-06-26 13:13:26 PDT
Benjamin - can you take a look?
Comment 10 Benjamin Smedberg [:bsmedberg] 2006-07-11 11:34:28 PDT
Created attachment 228831 [details] [diff] [review]
Check for colon, rev. 1
Comment 11 Benjamin Smedberg [:bsmedberg] 2006-07-11 11:35:40 PDT
This doesn't need to do a ".." check because nsStandardURL does those automatically in net_CoalesceDirs
Comment 12 Darin Fisher 2006-07-11 11:40:19 PDT
Comment on attachment 228831 [details] [diff] [review]
Check for colon, rev. 1

necko shouldn't depend on dom... use NS_ERROR_MALFORMED_URI instead, which is defined in nsNetError.h?

also, please add a comment explaining why a ':' in the file path is bad and why that protects us on Unix platforms.
Comment 13 Benjamin Smedberg [:bsmedberg] 2006-07-11 11:48:44 PDT
Created attachment 228835 [details] [diff] [review]
Check for colon, rev. 2
Comment 14 Benjamin Smedberg [:bsmedberg] 2006-07-12 08:27:50 PDT
Fixed on trunk.
Comment 15 Daniel Veditz [:dveditz] 2006-08-09 14:15:45 PDT
Comment on attachment 228835 [details] [diff] [review]
Check for colon, rev. 2

approved for 1.8.0 branch, a=dveditz for drivers
Comment 16 Benjamin Smedberg [:bsmedberg] 2006-08-10 12:24:19 PDT
Fixed on MOZILLA_1_8_0 branch.
Comment 17 alice nodelman [:alice] [:anode] 2006-09-07 11:56:02 PDT
I'm still able to load resource://// and resource:////home on Fedora (Fedora Core Release 5) using the latest 1.8.0.7 (20060906) and 1.8.1b2 (22060821).

Should I be seeing different behavior?
Comment 18 Benjamin Smedberg [:bsmedberg] 2006-09-07 12:07:10 PDT
Ugh, the site-absolute path still isn't protected.
Comment 19 Jay Patel [:jay] 2006-09-07 13:07:21 PDT
If those 2 paths still load, what exactly did the patch fix? 
Comment 20 Benjamin Smedberg [:bsmedberg] 2006-09-07 13:22:12 PDT
It fixed the resource:///http:// case
Comment 21 Benjamin Smedberg [:bsmedberg] 2006-09-08 08:30:08 PDT
Created attachment 237335 [details] [diff] [review]
Check for site-absolute path, rev. 1
Comment 22 Benjamin Smedberg [:bsmedberg] 2006-09-08 08:31:06 PDT
I'm trying to figure out how to write some unit tests for this.
Comment 23 Darin Fisher 2006-09-08 21:09:08 PDT
Comment on attachment 237335 [details] [diff] [review]
Check for site-absolute path, rev. 1

Add unit tests to netwerk/test/unit/

r=darin
Comment 24 Daniel Veditz [:dveditz] 2006-09-11 08:40:10 PDT
crud. I tested this on windows and was satisfied when resource:////c:/tmp/ no longer opened -- but that gets caught by the scheme-stripping because of the colon. If only I had tested the more correct resource:////c|/tmp/ which still shows the problem :-(
Comment 25 Benjamin Smedberg [:bsmedberg] 2006-09-11 08:51:25 PDT
Fixed on trunk.
Comment 26 Mike Beltzner [:beltzner, not reading bugmail] 2006-09-11 11:05:26 PDT
Comment on attachment 237335 [details] [diff] [review]
Check for site-absolute path, rev. 1

a=beltzner on behalf of 181drivers
Comment 27 Mike Beltzner [:beltzner, not reading bugmail] 2006-09-11 11:06:41 PDT
Dveditz: does comment 24 mean that this isn't fixed?
Comment 28 Daniel Veditz [:dveditz] 2006-09-11 11:11:21 PDT
We're not going to hold 1.8.0.7 for this, out to 1.8.0.8
Comment 29 Benjamin Smedberg [:bsmedberg] 2006-09-11 11:22:21 PDT
Created attachment 237784 [details] [diff] [review]
Unit test, rev. 1

I don't know that we want to check in the unit test until this has been publicly announced, since it makes the exploit perfectly obvious.
Comment 30 Dave Liebreich [:davel] 2006-09-11 11:29:29 PDT
Comment on attachment 237784 [details] [diff] [review]
Unit test, rev. 1

should we add a case for comment 24's case?

resource:////c|/tmp/
Comment 31 Benjamin Smedberg [:bsmedberg] 2006-09-11 11:37:34 PDT
Fixed (again) on MOZILLA_1_8_BRANCH
Comment 32 Darin Fisher 2006-09-12 10:30:34 PDT
Comment on attachment 237784 [details] [diff] [review]
Unit test, rev. 1

thanks for writing this testcase!
Comment 33 Bob Clary [:bc:] 2006-09-12 12:05:36 PDT
I added Benjamin's test + resource:////c|/tmp/ to my local security regression tests but am leaving the in-testsuite? open for davel. 

What other uris could we add to this check for completeness sake?
Comment 34 Daniel Veditz [:dveditz] 2006-09-19 15:36:29 PDT
Restoring lost blocking flag
Comment 35 Daniel Veditz [:dveditz] 2006-09-26 14:42:41 PDT
Comment on attachment 237335 [details] [diff] [review]
Check for site-absolute path, rev. 1

approved for 1.8.0 branch, a=dveditz for drivers
Comment 36 Benjamin Smedberg [:bsmedberg] 2006-09-27 11:19:00 PDT
Attachment 237335 [details] [diff] committed on MOZILLA_1_8_0_BRANCH
Comment 37 Benjamin Smedberg [:bsmedberg] 2006-10-25 12:35:57 PDT
Unit test landed on trunk.

Note You need to log in before you can comment on or make changes to this bug.