The default bug view has changed. See this FAQ.

resource:// can load any file on the disk

RESOLVED FIXED in mozilla1.8.1

Status

()

Core
Networking: File
P1
normal
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: bz, Assigned: bsmedberg)

Tracking

({fixed1.8.0.8, fixed1.8.1})

Trunk
mozilla1.8.1
x86
Linux
fixed1.8.0.8, fixed1.8.1
Points:
---
Bug Flags:
blocking1.9a1 +
blocking1.8.1 +
blocking1.8.0.4 -
blocking1.8.0.7 -
blocking1.8.0.8 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:low dos], URL)

Attachments

(3 attachments, 1 obsolete attachment)

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

Updated

11 years ago
Flags: blocking1.9a1?
Flags: blocking1.9a1+
Flags: blocking1.8.1?
Flags: blocking1.8.1+
Flags: blocking1.8.0.5?
Flags: blocking1.8.0.5+
(Assignee)

Comment 1

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

Comment 2

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

Comment 4

11 years ago
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://.
(Assignee)

Comment 6

11 years ago
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]

Comment 8

11 years ago
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+

Comment 9

11 years ago
Benjamin - can you take a look?
Assignee: nobody → benjamin

Updated

11 years ago
Target Milestone: --- → mozilla1.8.1beta2
(Assignee)

Updated

11 years ago
Status: NEW → ASSIGNED
Priority: -- → P1
(Assignee)

Comment 10

11 years ago
Created attachment 228831 [details] [diff] [review]
Check for colon, rev. 1
Attachment #228831 - Flags: review?(darin)
(Assignee)

Comment 11

11 years ago
This doesn't need to do a ".." check because nsStandardURL does those automatically in net_CoalesceDirs

Comment 12

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

Comment 13

11 years ago
Created attachment 228835 [details] [diff] [review]
Check for colon, rev. 2
Attachment #228831 - Attachment is obsolete: true
Attachment #228835 - Flags: review?(darin)

Updated

11 years ago
Attachment #228835 - Flags: review?(darin) → review+
(Assignee)

Comment 14

11 years ago
Fixed on trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Assignee)

Updated

11 years ago
Attachment #228835 - Flags: approval1.8.1?
Attachment #228835 - Flags: approval1.8.0.6?

Updated

11 years ago
Attachment #228835 - Flags: approval1.8.1? → approval1.8.1+
(Assignee)

Updated

11 years ago
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+
(Assignee)

Comment 16

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

Comment 18

11 years ago
Ugh, the site-absolute path still isn't protected.
Status: RESOLVED → REOPENED
Keywords: fixed1.8.0.7, fixed1.8.1
Resolution: FIXED → ---

Comment 19

11 years ago
If those 2 paths still load, what exactly did the patch fix? 
(Assignee)

Comment 20

11 years ago
It fixed the resource:///http:// case
(Assignee)

Comment 21

11 years ago
Created attachment 237335 [details] [diff] [review]
Check for site-absolute path, rev. 1
Attachment #237335 - Flags: review?(darin)
(Assignee)

Comment 22

11 years ago
I'm trying to figure out how to write some unit tests for this.

Comment 23

11 years ago
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+

Updated

11 years ago
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]
(Assignee)

Comment 25

11 years ago
Fixed on trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago11 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
(Assignee)

Updated

11 years ago
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+
(Assignee)

Comment 29

11 years ago
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.
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/
(Assignee)

Comment 31

11 years ago
Fixed (again) on MOZILLA_1_8_BRANCH
Keywords: fixed1.8.1

Comment 32

11 years ago
Comment on attachment 237784 [details] [diff] [review]
Unit test, rev. 1

thanks for writing this testcase!
Attachment #237784 - Flags: review?(darin) → review+

Comment 33

11 years ago
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+
(Assignee)

Comment 36

11 years ago
Attachment 237335 [details] [diff] committed on MOZILLA_1_8_0_BRANCH
Keywords: fixed1.8.0.8
Group: security
(Assignee)

Comment 37

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