209689 - document.cookie from hostless file: URL bypasses "ask me before storing"

Status: VERIFIED FIXED

(Core :: Networking: Cookies, defect)

Description

[benc]

Mach-O + Win98, Mozilla 1.4b.
(probably has happened for a long time).

STEPS:
Turn on "ask me before storing" cookies.
Open any file URL (to set the URL bar conext)
Type the provided URL in to the URL bar and hit return.

OBSERVED:
cookie is set (check cookie manager)

EXEPCTED:
"ask me" dialog should interupt.

I'm working on a js script that will progamatically determin if the DOM
permissions for cookies are on or off.

I noticed while developing this that the "ask me" dialog is never showing up.

This coupled with the fact that Cookie Manager seems to update every cookie in
the display EXCEPT the selected cookie, has actually made this script a pain to
develop.

But I'm more concerned about the problems w/ cookie security, because bug 109982
is won't fix. I'm going to look at that some more, finish writing the script,
and post it here.

I've also done a dupe check, and found now relevant, pre-existing bugs.

Comment 1

[[e-mail]]

hmm. 10 bucks says the check at
http://lxr.mozilla.org/seamonkey/source/extensions/cookie/nsCookiePermission.cpp#90
is going through, and we're just returning kDefaultPolicy (which allows the
cookie and skips the prompt):

 88   nsCAutoString hostPort;
 89   aURI->GetHostPort(hostPort);
 90   if (hostPort.IsEmpty())
 91     return NS_OK;

mvl... we need to fix this :)

Comment 2

[Darin Fisher]

feel free to take this bug (or any other) off my plate :)

Comment 3

[Michiel van Leeuwen (email: mvl+moz@)]

taking

Comment 4

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: patch v1

This is a quick fix. It makes it so that when you have a hostless url, it uses
the prePath to identify the "site", and uses that in the prompt. It also uses
that to ask the permission manager.
The dialog now reads: "The site file:// wants to set a cookie". It is a bit
strange, but the best I can think of now.

You could add file:// to cookperm.txt, but there is no UI for that. The menu's
don't do that yet, but that is another bug.

Comment 5

[Michiel van Leeuwen (email: mvl+moz@)]

Forget the comments on the permission manager, they are bogus. The patch only
affects the dialog, nothing else.

So, let me state it as a question: would using the prepath for the permission
list be ok when there is no host? That way, you could put file:// in cookperm.txt.

Comment 6

[Darin Fisher]

i think it'd be better to use the URI scheme instead of the prePath.  the reason
is that if the hostPort is empty, then that does not mean that the prePath
wouldn't possibly include a username and password.  we probably don't want to
include the username and password in key, so probably it makes most sense to
just compare schemes.  or perhaps you could just have a special entry for "empty
host"?

Comment 7

[[e-mail]]

mvl: are you going to roll a new patch to address darin's comments, or would you
like review on your current one?

Comment 8

[[e-mail]]

i agree with darin here... maybe using the scheme would be better?

r=dwitte with both that, and the spelling mistakes, fixed :)
(nuke that trailing newline too please. you could also kill that !aCookie test
if you want, since the only intended consumer is cookieservice, and it's quite
sane wrt errors)

Comment 9

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: patch v2

I changed it to hardcode :// after the scheme in the prompt. Just using the
scheme didn't work ("A site from file wants to set a cookie").

While testing, i noticed the checkbox didn't work. I fixed that to. It add an
entry "scheme:file" to cookperm.txt. I added scheme: to make sure it isn't a
host.
Now that i think of it, it might be a bit confusing. scheme:http will only
block hostless http cookies, not all http cookies.

I left the !aCookie check in, just to be sure :) I added another check, because
I was seeing assertions while testing. They occured because I hand-edited my
cookperm.txt, and messed up, but they were irritating enough to fix :)

Comment 10

[[e-mail]]

Comment on attachment 132433 [details] [diff] [review]
patch v2

seems ok to me...

Comment 11

[Darin Fisher]

Comment on attachment 132433 [details] [diff] [review]
patch v2

so, won't this break any permissions prefs for file:// URLs?  guess they were
already kind of horked.

please remove the printf!  maybe the GetSpec can go too?

seems like it might be good to write a helper function to get the "right"
hostport, no?

sr=darin

Comment 12

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: patch v3

Updated to darins comments.

Comment 13

[Michiel van Leeuwen (email: mvl+moz@)]

checked in.

Comment 14

[benc]

VERIFIED: Mozilla 1.6f, Mac OS X.

file: URLs w/o a host now trigger accept dialog in proper conditions (if accept
is turned on in prefs, and if "Cookie Sites" does not contain entry for scheme:file)

NOTE:
the null host (file:///) is still stored as blank in "stored cookies" (bug
file:///Users/benc/Public/mozilla-org/html/quality/networking/testing/cookies/jscookietest.html)

Also, because of bug file://<hostname>/ can still write to any cookie (bug
109982), file  URL's can stomp on the "scheme:file" entry. I've filed bug 235170
on this.