Closed Bug 337744 Opened 19 years ago Closed 18 years ago

resource:// can load any file on the disk

Categories

(Core :: Networking: File, defect, P1)

x86
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla1.8.1

People

(Reporter: bzbarsky, Assigned: benjamin)

References

()

Details

(Keywords: fixed1.8.0.8, fixed1.8.1, Whiteboard: [sg:low dos])

Attachments

(3 files, 1 obsolete file)

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.
Flags: blocking1.9a1?
Flags: blocking1.8.1?
Flags: blocking1.8.0.5?
Flags: blocking1.9a1?
Flags: blocking1.9a1+
Flags: blocking1.8.1?
Flags: blocking1.8.1+
Flags: blocking1.8.0.5?
Flags: blocking1.8.0.5+
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
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.
Flags: blocking1.8.0.4?
does it handle escaped dots as well?
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?
<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://.
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
Filed bug 338037 on the chrome: url issue in comment 6 and 2. Let's stick to the resource: problem here.
Whiteboard: [sg:moderate]
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.
Flags: blocking1.8.0.4? → blocking1.8.0.4-
Flags: blocking1.8.0.5+ → blocking1.8.0.6+
Benjamin - can you take a look?
Assignee: nobody → benjamin
Target Milestone: --- → mozilla1.8.1beta2
Status: NEW → ASSIGNED
Priority: -- → P1
Attached patch Check for colon, rev. 1 (obsolete) — Splinter Review
Attachment #228831 - Flags: review?(darin)
This doesn't need to do a ".." check because nsStandardURL does those automatically in net_CoalesceDirs
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.
Attachment #228831 - Flags: review?(darin) → review-
Attachment #228831 - Attachment is obsolete: true
Attachment #228835 - Flags: review?(darin)
Attachment #228835 - Flags: review?(darin) → review+
Fixed on trunk.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Attachment #228835 - Flags: approval1.8.1?
Attachment #228835 - Flags: approval1.8.0.6?
Attachment #228835 - Flags: approval1.8.1? → approval1.8.1+
Keywords: fixed1.8.1
Comment on attachment 228835 [details] [diff] [review] Check for colon, rev. 2 approved for 1.8.0 branch, a=dveditz for drivers
Attachment #228835 - Flags: approval1.8.0.7? → approval1.8.0.7+
Fixed on MOZILLA_1_8_0 branch.
Keywords: fixed1.8.0.7
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?
Ugh, the site-absolute path still isn't protected.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
If those 2 paths still load, what exactly did the patch fix?
It fixed the resource:///http:// case
Attachment #237335 - Flags: review?(darin)
I'm trying to figure out how to write some unit tests for this.
Comment on attachment 237335 [details] [diff] [review] Check for site-absolute path, rev. 1 Add unit tests to netwerk/test/unit/ r=darin
Attachment #237335 - Flags: review?(darin) → review+
Status: REOPENED → ASSIGNED
Target Milestone: mozilla1.8.1beta2 → mozilla1.8.1
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 :-(
Flags: blocking1.8.0.8?
Whiteboard: [sg:moderate] → [sg:low dos]
Fixed on trunk.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago18 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Attachment #237335 - Flags: approval1.8.1?
Attachment #237335 - Flags: approval1.8.0.8?
Comment on attachment 237335 [details] [diff] [review] Check for site-absolute path, rev. 1 a=beltzner on behalf of 181drivers
Attachment #237335 - Flags: approval1.8.1? → approval1.8.1+
Dveditz: does comment 24 mean that this isn't fixed?
We're not going to hold 1.8.0.7 for this, out to 1.8.0.8
Flags: blocking1.8.0.8?
Flags: blocking1.8.0.8+
Flags: blocking1.8.0.7-
Flags: blocking1.8.0.7+
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.
Attachment #237784 - Flags: review?(darin)
Comment on attachment 237784 [details] [diff] [review] Unit test, rev. 1 should we add a case for comment 24's case? resource:////c|/tmp/
Fixed (again) on MOZILLA_1_8_BRANCH
Keywords: fixed1.8.1
Comment on attachment 237784 [details] [diff] [review] Unit test, rev. 1 thanks for writing this testcase!
Attachment #237784 - Flags: review?(darin) → review+
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?
Restoring lost blocking flag
Flags: blocking1.8.0.8?
Flags: blocking1.8.0.8? → blocking1.8.0.8+
Comment on attachment 237335 [details] [diff] [review] Check for site-absolute path, rev. 1 approved for 1.8.0 branch, a=dveditz for drivers
Attachment #237335 - Flags: approval1.8.0.9? → approval1.8.0.8+
Attachment 237335 [details] [diff] committed on MOZILLA_1_8_0_BRANCH
Keywords: fixed1.8.0.8
Group: security
Unit test landed on trunk.
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: