Closed Bug 491801 (CVE-2009-1835) Opened 15 years ago Closed 15 years ago

Arbitrary domain cookie access from content loaded via local file (file:/// URL)

Categories

(Core :: Networking, defect)

PowerPC
All
defect
Not set
normal

Tracking

()

VERIFIED FIXED

People

(Reporter: gfleischer+bugzilla, Assigned: dveditz)

References

Details

(Keywords: verified1.8.1.22, verified1.9.0.11, verified1.9.1, Whiteboard: [sg:moderate])

Attachments

(2 files)

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
Attached file Test case
Save test case locally and open.  Entry arbitrary domain to test.

Domain and cookie (if any) should be displayed.
Very nice.
Assignee: nobody → dveditz
Status: UNCONFIRMED → NEW
Component: General → Security: CAPS
Ever confirmed: true
Flags: wanted1.9.0.x+
Flags: blocking1.9.0.12+
Flags: blocking1.9.0.11?
Product: Firefox → Core
QA Contact: general → caps
Whiteboard: [sg:moderate]
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.
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.
Attachment #376359 - Flags: superreview?(cbiesinger)
Attachment #376359 - Flags: review?(jduell.mcbugs)
Component: Security: CAPS → Networking
QA Contact: caps → networking
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 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.
Attachment #376359 - Flags: review?(jduell.mcbugs) → review+
Attachment #376359 - Flags: superreview?(cbiesinger) → superreview+
Comment on attachment 376359 [details] [diff] [review]
ignore auth in a file:/// uri

Seems fine to me.
Flags: blocking1.9.1?
Attachment #376359 - Flags: approval1.9.1?
Attachment #376359 - Flags: approval1.9.0.11?
Comment on attachment 376359 [details] [diff] [review]
ignore auth in a file:/// uri

Approved for 1.9.0.11. a=ss for release-drivers
Attachment #376359 - Flags: approval1.9.0.11? → approval1.9.0.11+
Flags: blocking1.9.0.12+
Flags: blocking1.9.0.11?
Flags: blocking1.9.0.11+
Flags: blocking1.9.1? → blocking1.9.1+
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
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x?
Keywords: fixed1.9.0.11
Flags: blocking1.8.1.next?
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.
Attachment #376359 - Flags: approval1.9.1?
http://hg.mozilla.org/mozilla-central/rev/bff114502666
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/43de7d34998a
Status: NEW → RESOLVED
Closed: 15 years ago
Keywords: fixed1.9.1
Resolution: --- → FIXED
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.
Status: RESOLVED → VERIFIED
Comment on attachment 376359 [details] [diff] [review]
ignore auth in a file:/// uri

It applies fine to 1.8.
Flags: blocking1.8.1.next? → blocking1.8.1.next+
Attachment #376359 - Flags: approval1.8.1.next?
Comment on attachment 376359 [details] [diff] [review]
ignore auth in a file:/// uri

Approved for 1.8.1.22. a=ss for release-drivers
Attachment #376359 - Flags: approval1.8.1.next? → approval1.8.1.next+
Checked into 1.8 branch.
Keywords: fixed1.8.1.22
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.
Group: core-security
Alias: CVE-2009-1835
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)
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);
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
Regressions: 1634835
You need to log in before you can comment on or make changes to this bug.