491801 - (CVE-2009-1835) Arbitrary domain cookie access from content loaded via local file (file:/// URL)

Status: VERIFIED FIXED

(Core :: Networking, defect)

Description

[Gregory Fleischer]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.4; en-US; rv:1.9.0.10) Gecko/2009042315 Firefox/3.0.10
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.4; en-US; rv:1.9.0.10) Gecko/2009042315 Firefox/3.0.10

Content loaded from a local file (file:/// URL) can read cookies from arbitrary domains.

The domain name is calculated from the host portion of the URL, but the host name is ignored when loading content.

For example, file://example.com/C:/foo.html will load foo.html from the local disk, but the domain will be example.com.

If Firefox is configured as the default browser, remote HTML content served with "Content-Disposition: attachment" will be saved locally before being opened.  When opened, the content can load itself and specify an arbitrary domain for the host name.  Any currently readable cookies for the domain can be retrieved via 'document.cookie'.


Reproducible: Always

Comment 1

[Gregory Fleischer]

Attachment: Test case

Save test case locally and open.  Entry arbitrary domain to test.

Domain and cookie (if any) should be displayed.

Comment 2

[Daniel Veditz [:dveditz]]

Very nice.

Comment 3

[Daniel Veditz [:dveditz]]

This appears to be an ancient bug: the "noAuth" url parser goes ahead and parses the auth section

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/base/src/nsURLParsers.cpp&rev=1.32&mark=422,426#392

Easy to fix by setting the auth length to 0 (ignore auth) or returning an error, but am I missing some important reason it's this way? Elsewhere we prevent callers from adding userinfo or host to a pre-existing file URI

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/base/src/nsStandardURL.cpp&rev=1.111&mark=1343,1347#1334

There's no way to signal error from ParseAfterScheme. If we set the auth section to 0 there we'll just silently ignore any host used in a file URI (the URI will get rewritten without the host). In terms of loading files that's what we're doing, so we won't break anyone who has accidentally included a host bit.

Or we could leave the auth bits in ParseAfterScheme and then override ParseAuthority to return an error, making any such URI invalid.

Comment 4

[Daniel Veditz [:dveditz]]

Attachment: ignore auth in a file:/// uri

This patch silently drops an authority section in a file:/// uri. The added ParseAuthority() is paranoia/future-proofing.

If we want to go the other way we can drop the ParseAfterScheme change and then the error returned from ParseAuthority will fix the bug by invalidating any file: URI with an auth section.

Safari leaves the ignored auth in the URL like we do (but unlike us doesn't use it as a host).

IE (8) treats file:// the same as file:///, it does not treat the bit in-between as an authority.

I guess I lean toward this patch rather than the error because it's less likely to break things. If we want the stricter form for mozilla-central I can do that, too.

Comment 5

[Daniel Veditz [:dveditz]]

One warning sign, as part of bug 110560 (attachment 59282 [details] [diff] [review]) Darin changed a later part of nsNoAuthURLParser::ParseAfterScheme because of cookie crashes.

-    SET_RESULT(auth, 0, -1);
+    SET_RESULT(auth, pos, 0);

Should I worry about having used SET_RESULT(auth, 0, -1) a few lines earlier? It's not causing me any problems that I've noticed and a lot has changed since 2001, but that was a somewhat obscure crash.

Comment 6

[Jason Duell]

Comment on attachment 376359 [details] [diff] [review]
ignore auth in a file:/// uri

Looks good to me.  As I discussed with you, we're not taking away any functionality with this patch--just plugging the security hole.  If someone really wants "file://foo.com/c:\whatever" to work, they can ask for it in Bugzilla.

> Should I worry about having 
>used SET_RESULT(auth, 0, -1)

I don't know.  {0,-1} is the default constructed state of a URLSegment.  I can't think of why it would be a problem (but trying to trace through all the touched code woudl be a bear).  I suggest we let this bake on the trunk for a little while before patching for 1.9.0.x.

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 376359 [details] [diff] [review]
ignore auth in a file:/// uri

Seems fine to me.

Comment 8

[Samuel Sidler (old account; do not CC)]

Comment on attachment 376359 [details] [diff] [review]
ignore auth in a file:/// uri

Approved for 1.9.0.11. a=ss for release-drivers

Comment 9

[Daniel Veditz [:dveditz]]

Fixed for 1.9.0
Checking in nsURLParsers.cpp;
/cvsroot/mozilla/netwerk/base/src/nsURLParsers.cpp,v  <--  nsURLParsers.cpp
new revision: 1.33; previous revision: 1.32
done
Checking in nsURLParsers.h;
/cvsroot/mozilla/netwerk/base/src/nsURLParsers.h,v  <--  nsURLParsers.h
new revision: 1.5; previous revision: 1.4

Comment 10

[Johnny Stenback (:jst)]

Comment on attachment 376359 [details] [diff] [review]
ignore auth in a file:/// uri

This is a 1.9.1 blocker, no need for explicit approval. Please land asap.

Comment 11

[Daniel Veditz [:dveditz]]

http://hg.mozilla.org/mozilla-central/rev/bff114502666
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/43de7d34998a

Comment 12

[Al Billings [:abillings - ex-MoCo]]

Verified for 1.9.0.11 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.11pre) Gecko/2009051804 GranParadiso/3.0.11pre using testcase.

Verified for 1.9.1 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b5pre) Gecko/20090518 Shiretoko/3.5b5pre.

Verified in Trunk with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090517 Minefield/3.6a1pre.

Comment 13

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 376359 [details] [diff] [review]
ignore auth in a file:/// uri

It applies fine to 1.8.

Comment 14

[Samuel Sidler (old account; do not CC)]

Comment on attachment 376359 [details] [diff] [review]
ignore auth in a file:/// uri

Approved for 1.8.1.22. a=ss for release-drivers

Comment 15

[Daniel Veditz [:dveditz]]

Checked into 1.8 branch.

Comment 16

[Al Billings [:abillings - ex-MoCo]]

Verified for 1.8.1 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.22pre) Gecko/20090602 SeaMonkey/1.1.17pre. Testcase causes bug with previous Seamonkey release.

Comment 17

[Anders]

This breaks pop-up "allowing"

My homepage is file://localhost/c:/home.html and this site can open pop-ups on different keystrokes, for this to work, localhost is whitelisted in the pop-up blocker. With this patch, file://localhost/... is redirected to file:///... and when a pop-up is blocked there, the "allow pop-ups for" menu item on the info-bar is broken, it does nothing. I suggest that if the domain is localhost, the redirection should be turned off (Or let me allow pop-ups for file://* in the pop-up blocker)

Comment 18

[Stefan]

I'd been using the IE View plugin to forward file:// urls to explorer.exe so that links to samba shares could be opened from the browser. It seems that this fix breaks that solution. To remedy this, I've ended up with what seems to be a cleaner solution. I've switched my links to use smb:// style urls, and registered wscript (windows scripting host) to handle the smb: protocol with the following script (smb.js):

var arg = WScript.Arguments.Item(0);
var loc = arg.replace(/^smb:/,"file:");
shell = WScript.CreateObject("WScript.Shell");
shell.Run(loc);

Comment 19

[Daniel Veditz [:dveditz]]

On Windows file:////share/path will open samba links.

> (Or let me allow pop-ups for file://* in the pop-up blocker)

That's bug 204285