337744 - resource:// can load any file on the disk

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

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]

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]

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)]

does it handle escaped dots as well?

Comment 4

[Darin Fisher]

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 [:bzbarsky]]

  <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]

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]]

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]]

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]

Benjamin - can you take a look?

Comment 10

[Benjamin Smedberg]

Attachment: Check for colon, rev. 1

Comment 11

[Benjamin Smedberg]

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

Comment 12

[Darin Fisher]

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]

Attachment: Check for colon, rev. 2

Comment 14

[Benjamin Smedberg]

Fixed on trunk.

Comment 15

[Daniel Veditz [:dveditz]]

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]

Fixed on MOZILLA_1_8_0 branch.

Comment 17

[alice nodelman [:alice] [:anode]]

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]

Ugh, the site-absolute path still isn't protected.

Comment 19

[Jay Patel [:jay]]

If those 2 paths still load, what exactly did the patch fix? 

Comment 20

[Benjamin Smedberg]

It fixed the resource:///http:// case

Comment 21

[Benjamin Smedberg]

Attachment: Check for site-absolute path, rev. 1

Comment 22

[Benjamin Smedberg]

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

Comment 23

[Darin Fisher]

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]]

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]

Fixed on trunk.

Comment 26

[Mike Beltzner [:beltzner, not reading bugmail]]

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]]

Dveditz: does comment 24 mean that this isn't fixed?

Comment 28

[Daniel Veditz [:dveditz]]

We're not going to hold 1.8.0.7 for this, out to 1.8.0.8

Comment 29

[Benjamin Smedberg]

Attachment: 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]]

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]

Fixed (again) on MOZILLA_1_8_BRANCH

Comment 32

[Darin Fisher]

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

thanks for writing this testcase!

Comment 33

[Bob Clary [:bc] (inactive)]

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]]

Restoring lost blocking flag

Comment 35

[Daniel Veditz [:dveditz]]

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]

Attachment 237335 [details] [diff] committed on MOZILLA_1_8_0_BRANCH

Comment 37

[Benjamin Smedberg]

Unit test landed on trunk.