Closed Bug 22994 Opened 25 years ago Closed 22 years ago

Mail reader allows spammers to set cookies to track web usage

Categories

(MailNews Core :: Security, defect, P2)

Product:
MailNews Core
Component:
Security
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
mozilla1.4beta

People

(Reporter: nick255, Assigned: darin.moz)

References

(Blocks 1 open bug)

Details

(Whiteboard: [adt2] [not blocking css files])

Attachments

(8 files, 2 obsolete files)

screenshot of the mail cookie pref (default value)
49.14 KB, image/jpeg
Details
Example1
32.42 KB, image/gif
Details
Example2
32.56 KB, image/gif
Details
Example3
31.84 KB, image/gif
Details
a 'proactive description' for the mail news cookie pref
31.76 KB, image/gif
Details
proposed fix, v2
10.49 KB, patch
dveditz
: review+
dveditz
: superreview+
asa
: approval+
Details | Diff | Splinter Review
screenshot w/ p3p (modern skin)
52.28 KB, image/jpeg
Details
v1 patch
29.86 KB, patch
dwitte
: review+
dveditz
: superreview+
sspitzer
: approval1.4b+
Details | Diff | Splinter Review
the Mozilla mail reader allows someone to send you an email that allows a cookie to be set. this can be used by ad banner sites, such as doubleclick, to send bulk email that can track your web surfing. this bug has been featured on slashdot.org, junkbusters.com, and even eff.org.
Status: NEW → ASSIGNED
Target Milestone: M13
nick255, it would be really helpful if you can provide an example how spammers do the trick. Before and after. Thanks much.
I believe it uses either a set-cookie in the headers, or javascript to set a cookie. It probably uses a set-cookie in javascript, but the bug is in almost every HTML based email client.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → DUPLICATE
Target Milestone: M13 → M14
M14 ... I need a test case for this. *** This bug has been marked as a duplicate of 16964 ***
I don't beleive that this is exactly a duplicate of 16964. The problem results from some email that has an img in it. The server sends a cookie in the start of the http response, along with the usual things such as "Content-type: image/gif, Content-Length: 5222" etc. I wrote a quick and simple example. Go to http://www.turnstep.com/magic/cs2 and fill in your email address. The script will email you a message that contains an image. The image will set a cookie named "pizza" when the email is read in a HTML-aware browser. Go back to the page to see if the cookie is now recognized by the site. What's needed is something that checks if the user is in mail or news while the image (or any http request?) is being loaded, and if they are, the cookie is not loaded at all. This could be done at a higher level than nscookie.cpp, but I haven't found a better place in the code yet. For further testing, you can also view, add, or delete any cookies related to my site here: http://www.turnstep.com/magic/cook (All URLs above are for mozilla testing purposes only) On second thought, this might be a duplicate, but the exact nature of the other bug is not clear yet. I didn't think your email addresss was an object accessible by javascript.
Status: RESOLVED → REOPENED
Reopening the bug and thanks much for cooking up an example. I'll look into it. Appreciated.
Resolution: DUPLICATE → ---
Status: REOPENED → ASSIGNED
No, your email address was included as the text (data) in the cookie.
greg, your example seems not work as I would expected. The example requires user's input in order to get the user's email address. I am more interested in no user's action required to allow malicious html message to automatic record user's email address eiter by setting cookies or posting the information to the web server.
The way I understand it, the spammer's mass emailer creates the set-cookie command with the data containing the email address of the person being sent to (i.e. the cookie code is generated dynamically for each spam). And then the site(s) are informed as to how to retrieve the cookie (i.e. ad sites "rent" space on the spammer's program).
As I understand it, this is how it works: 1) SPAM is generated with an embedded image. The img src URI has a query appended to it (eg, http://www.spammer.org/image.gif?your@email.com). 2) You receive the message. When you open it, the mail reader requests the image along with the query string. 3) Now the spammer has a list of confirmed email addresses. Not only that, but the server can be used to set a cookie to track the effectiveness of the spam. Through the use of cookies and HTML, the spammer has an effective tool for gathering a list of valid email addresses and tracking which of those addresses actually visit the site being spammed and how often. This presents a real problem as there are spamming programs out there that generate random userid's and append a domain. This bug allows them to winnow the good email addresses that result from this activity.
Adding myself to the cc list.
http://www.turnstep.com/magic/cs2 I think I can clear up some confusion. The URL example I gave has an email request field just to get the ball rolling. Just make believe that the mail you receive as a result of that form came from a spammer, and that you did not request it. When the message is read in a mail viewer, it calls an IMG that has a id number in the query_string (nothing so obvious as an email address, of course). When the browser/mail reader requests the image, my server actually does three things: 1) Send the image 2) Sets a cookie 3) Logs a successful reading of the message into an internal database. You can then go to the same URL again: it will detect your cookie, which was set by the mailer, and tell you what your email is, as well as some other things. The bottom line is, cookies should only be set when people are actually browsing. I rewrote both scripts today, let me know if I broke them! :)
Thanks much for more info, greg & jerrybaker. I haven't get a chance to look into the bug yet. I'll soon.
Would like to be able to do something about this for beta1. It seems like it hinges on being able to disable cookie responses for mail messages. cc'ing morse. Steve, any pointers here?
What really needs to happen is that mail needs to prohibit retrieving anything from the network without asking the user. Getting rid of cookies only kills the tracking problem, but it does not stop the spam harvesting at all. All they need is the embedded image with a unique query string.
Sorry for getting in on this discussion late but I just got cc'ed on this today. Yes, it absolutely is a dup of 16964. And, as I mentioned in that but report, the problem is solved in 5.0 much better than in 4.x because of the improved semantics for don't-accept-foreign-cookies as implemented in 5.0. Checking for being in mailnews when the cookie is being set is not an acceptable solution because it is incomplete. It would not do anything at all for webmail.
Multiple comments... Hrmm....well, it IS a duplicate of 16964, but only because the original question of 16964 was changed...the original bug reporter claimed that his email was sent by merely clicking on a link, not mail-related at all. I think that bug should be closed, as I know of no way outside this email loophole for someone to get your email address. 'jerrybaker' is correct: cookies are not necessarily needed to verify legitimate email addresses. Don't see a simple way around that without denying almost all IMG SRC in email - not something I ever use, but... What do you mean by "it would not do anything for webmail?" Finally, I feel this should not be linked to a pref, but "hard-coded" to deny all cookie setting and reading when not in the browser.
The means of tracking by putting query strings into urls in mail messages is an old story. We spoke about that years ago. There is no defense against it. Let's not confuse this bug report with that issue. Here's what I meant by my webmail comment. The same sort of mis-use of cookies that you are concerned about here for messenger mail applies to webmail as well. But the solution you are advocating (don't allow cookies when in mail, don't allow images when in mail, etc) will have no effect in webmail. When you read webmail you are using the general browser for viewing websites and not running a specific client-based mail program. Once you consider webmail and realize that you can't special-case things for the mail program, you then realize that your only line of defense is a preference setting. And we've had the no-foreign-cookie pref from day one, exactly for this sort of thing. So it indeed should be the solution that we tell users about when they are concerned about this attack. Especially now that we have fixed up the semantics of the foreign cookie pref (see discussion in bug 16964 about that).
It would sure help for newsgroups, as the major news sites (deja.com) don't allow downloading of images.
If by webmail you mean web-based email services such as hotmail and bigfoot, then that is their problem, and not mozilla's. We should close the mail cookie loophole forever, for there really is no legitimate use I can think of to leave it enabled, ever, pref or no pref.
No, I differ with you. It is the same problem whether it is webmail or it is messenger mail. The correct solution is to use the foreign-cookie pref. It solves it perfectly fine for both.
The webmail is a red herring: how a browser is used is not the concern. Equating mail and browsing is wrong: with mail, you have no control over what you receive, with browsing, you do. Even if sites have images (and therefore cookies) from another site, the underlying code to do so is the responsibility of the site you manually visit. If I go to yahoo.com, I can be assured that I will not see a pornographic banner ad with a cookie attached. If I go to eff.org, I can be sure that there will not be a doubleclick banner trying to track me. If I open mail, I can receive an email from a spammer, with an image from anywhere on the web, complete with nasty cookies. Therefore, the two are fundamentally different. If someone does not want foreign cookies in email (a normal assumption) and does want it while browsing (also a normal assumption), why should they have to change their prefs everytime they switch from one to the other? No other prefs that I know of overlap like this, nor should they. I recommend leaving the foreign-cookie pref for browsing, and simply treat the mail reader as "no cookies ever"
There's some confusion here about webmail. I'm not talking about the images and cookies put up by the webmail site. I'm talking about the images and cookies embedded in the mail message that you've received. Makes no difference if that mail message was received by a mail program embedded in the browser or a mail program running out on the web. It's the same problem with the same privacy violation. For the sake of this discussion, consider that we have a webmail site that you yourself developed. No ads on the site. No foreign images. And no cookies.
Doesn't matter, Web mail is just what it says - *WEB*. If you do not want *WEB*, then don't use Web mail. Cookies are part of the Web, not part of email.
I'm afraid I'll never be able to convince you so I'll stop trying.
I am not talking about webmail sites like hotmail or yahoo mail. I am talking about the email client (and news client) in mozilla that is used to access POP3/IMAP email servers and NNTP news servers. You don't see Eudora Light or Free Agent setting cookies, do you?
stepping in because I have to agree with this bug. Adding norris for security and warren/dp for http/cookie support. I think we should be disabling cookies when the principle of an HTML page is not an HTTP url... in the case of mail, the principle is something like mailbox://blahblah, so if a message has <img src="http://...."> then the channel created by that should have cookies disabled.
Should disable foreign content at all if the user chooses. Getting rid of cookies does not prevent harvesting email addresses.
Bug #28327 "No server hits for at mailnews reading" is related.
Adding Steve Morse to cc list. Steve, my recollection from the conversation with Richard Smith about this topic is that we were protected in mozilla. Does it require setting the "ask me before setting cookies pref"?
Norris, I already was on the cc list. See all my comments above. After I made my point, I bowed out of the conversation because there was nothing more that I could add. The solution that we told Richard Smith and the press was to use the reject-foreign-cookie pref (no need for the ask-me-before-setting-cookies pref). That way we completely solved the problem for both messenger mail and web-mail, and there is no reason to consider these as different beasts. Richard Smith was happy with that solution and his only objection was that in 4.7 the pref blocked the setting of foreign cookies but not the sending of foreign cookies. He gave some examples of how just the sending could lead to privacy intrusions. So I modified the cookie code in seamonkey to block it in both directions when the pref is enabled.
And is the reject-foreign-cookie pref ON by default ?
No, it's not. We might want to consider setting it that way in seamonkey with the caveat that we might break some as-of-yet unknown site by doing so.
Here is what I think. My baseline is that we *HAVE TO* solve this for mail client. Solving it for webmail is next priority (in no way low) for me. So if we agree to turn this reject-foreign-cookie ON by default, that solves both problems and all of us are happy. If not, then I would like to entertain a mail only solution of not sending cookies on images viewed in mail. Steve, looks from earlier comments that you weren't averse to this solution. Just that you were more championing the bigger solution of using the reject-foreign-cookie pref. Can we make a decision. Personally setting the pref ON by default seems pretty aggressive to me for the reason you mentioned. But I will let you make the call.
Well if it's my call, then I'll definitely go for setting the pref on by default. It's the way I always set my browser anyway. I'll make the change.
I think we should set it ON by default, because I think user's privacy is more important than whether or not they can read their egghead.com weekly specials e-mail. We should definately release note it though. I'm sure that if we put this out in beta, most of the spammers will get smart and start using images-in-MIME (whatever that's called)
jar vetoed setting it on by default in the beta and has reservations about ever doing so.
that is LAME. we can turn this on by default in mozilla and off by default for netscape, and as a mozilla contributor, I think we should do that.
<sarcasm> Glad to see that Netscape is still up to the challenge of demonstrating what a huge concern their users' privacy is. </sarcasm>
Alec: I agree Jerry: *g*
Why is it so hard to grasp? There is *no reason* to involve cookies with a mail program, period. Not a single legitimate use exists. Setting it as a preference is absurd.
Wait let us not jump on opinions. I see issues with doing the reject-foreign-cookie ON by default for beta1. It is certainly risky. Now if we can do a simple solution for mail only, I dont think that would be rejected too. That is more local and less risky to break browsing habits. Doing something just for mozilla is not too useful. Most of the userbase will be running netscape. And we should be looking out for the user.
It's simple. Disconnect mail from Necko. Easy, done, fixed.
I don't understand why this is such a hot issue. The pref exists and it does exactly what is needed (more than it did in 4.x). The only issue now is the default setting. For those who know enough about it to care, they'll set it to no-foreign-cookies. For those that don't know enough about it, they couldn't care less what it is set to.
This is such a hot issue because the Mozilla/Netscape people are apparently failing to understand that users are sick and tired of SPAM. They are sick and tired of software companies ignoring their privacy concerns. Mozilla/Netscape apparently is very resistant to the idea of making it impossible to import content into Messenger from the Internet. Anything other than the ability to disconnect Mail from retrieving _anything_ is just hand waiving on the part of Mozilla & Netscape. People are tired of it. That's why it's such a hot issue.
Jerry, Mailnews won't work without Necko. Bug #28327 is, what you want.
Steve, it is ridiculous to say that non-technical people don't care about their privacy. Of course they care what it is set to. If it is set during beta1 at least we can see what affect it has. So far I haven't heard one instance of a known site this breaks. Why wait till later when there are even more people using it? You can be sure that if Netscape has a privacy option in the beta and turns it off by default I will personally let everyone know they don't care about their privacy.
> You can be sure that if Netscape has a privacy option in the beta and turns it > off by default I will personally let everyone know they don't care about their > privacy. IIRC, Communicatior 4.x has this option for a long time now, and it was always off by default.
Just keep the following in mind before you conclude that Netscape (and I in particular) don't care about user privacy: - Netscape has the reject-foreign-cookie pref whereas IE does not. - I improved that pref in 5.0 to close the loophole that Smith reported. - I implemented the ability to automatically reject cookies from selected sites. - I fought (and won) the battle to store the user's wallet data on the client rather than uploading it to netcenter. And I'll continue to fight such battles, but you need to pick your fights carefully.
Email presents a much larger problem than the Web. A reasonably informed user knows that Web sites set cookies, and that Web sites can get information such as IP address, etc. Most users are not aware that by simply sending mail, a spammer can set cookies. Worse than that IMHO is the fact that spammer can pick a domain and send random email (eg, aaaa@netscape.com -> zzzzz@netscape.com) and then get unavoidable confirmation of the valid email addresses simply by including a link to an image with a unique query string. It is only a matter of time before some spam program includes this feature built-in. It makes me rather angry that people at Mozilla and Netscape are unconcerned with this. I want to be in control of what happens with my mail, not some spammer.
Please, the usefulness of the bugreport is going down. May I suggest a newsgroup for the above. Bottomline, we would like to have a mail only solution. Do we have one ? If not, let us wait until we evaluate the pref option for beta2.
> I fought (and won) the battle to store the user's wallet data on the client > rather than uploading it to netcenter. Uh! That would be such an unbelievable privacy violation, that I think, it could reduce Navigator's market share. I value your efforts and that of many other developers, who fight Netscape's marketing devision.
This is a big deal. Most of the discussion seems to be focused on the cookie aspect, but not the image URL aspect... This URL contains big information on this entire issue: http://www.tiac.net/users/smiths/privacy/cookleak.htm also take a look at these for more info. http://www.idg.net/crd_sites_9-46489.html http://www.tiac.net/users/smiths/privacy/wbfaq.htm http://tbtf.com/blog/1999-10-24.html#20
icos, this bug *is* about cookies. Again: See bug #28327.
> IIRC, Communicatior 4.x has this option for a long time now, and it > was always off by default. If this was in NC4, and it was off by default, then it had no UI, and I never heard about it, which just leaves me speechless. It may as well have not been there and just reinforces what I said. Was this there before the privacy problem became big news sometime last year? Was someone aware of the issue before then? > - Netscape has the reject-foreign-cookie pref whereas IE does not. I don't care about IE, I expect nothing good from Microsoft. > - I improved that pref in 5.0 to close the loophole that Smith reported. Which is 95% useless if it's not on by default. > - I implemented the ability to automatically reject cookies from > selected sites. Is prompting behaviour on by default? Otherwise, it doesn't help novices. If it was, it would basically remove the need for this (although you could still make an argument there's no need to allow the cookies). > - I fought (and won) the battle to store the user's wallet data on the > client rather than uploading it to netcenter. The fact that you had to fight anything is evidence of Netscape-in-general's lack of commitment to privacy.
matty, as ong as I didn't mix anything up, it's "network.cookie.cookieBehavior"=1 in 4.5, and enabled with Prefs|Advanced|Cookies|Accept only cookies ... originating server.
Bulk moving all MailNews Security bugs to new Security: General component. The previous Security component for MailNews will be deleted.
Component: Security → Security: General
Sorry, my fault. I realised after I wrote that was it was all about. Was this pref there for that reason? I don't consider it "the same pref" since the mail is the important bit as far as I'm concerned, but I should have been paying more attention.
It was there for the purpose of maintaining privacy, whether it be while reading mail or while browsing. It was not a knee-jerk reaction to the attack that Richard Smith disclosed as it existed a long time before. Therefore it was an ideal response from Netscape to that attack since it was protecting the user from exactly what it was supposed to protect him from.
Sorry to but in, but I've been reading all the comments on this bug with interest. How about making it _part of the install_ to actually prompt the user 'What cookie setting do you want: accept all cookies, accept cookies only from the site you are browsing, do not accept cookies'. That would seem to leave everyone happy.
Installer option: Don't know if we'd want to burden the user with this option on install, but in either case I doubt we'd get this question into beta 1, and by beta 2 maybe people will find out whether it causes any problems (or separate the mail and web solution).
Target Milestone: M14 → M18
Adding myself to CC: list due to related discussion in bug 28327.
Blocks: 40756
reassigning jefft's bugs to naving
Assignee: jefft → naving
Status: ASSIGNED → NEW
*** Bug 63525 has been marked as a duplicate of this bug. ***
Target Milestone: M18 → ---
Keywords: mozilla0.9
What I am doing is using 4.x just for mail. I also set the HTTP proxy to localhost:666 (except for local servers). This solves all privacy problems nicely. Setting up 2 copies of mozilla would solve the problem too if it were possible to run with 2 profiles at once (I believe it is on linux, probably not on windows).
you can run multiple instances of most apps including mozilla on windows nt. but for 9x you might as well use mfcembed/winembed for browsing and mozilla for mail (seperate profile for each) -- assuming you don't need any of the navigator chrome... hrm... does pac have a way to discriminate between mail and browsing?
timeless: PAC is cannot do that, it is basically a function that provides a routing table via javascript code. You could certainly enhance it to have modes, perhaps an RFE you could create for what brendan and I have jokingly called PacNG or PAC2.
If no one objects, I'm taking this bug. It hasn't seen any action in months, and I think it still deserves some serious consideration. Rejecting third-party cookies does not prevent cookies from being set by images or other included content in a mail message, does it? And the fact that offering the option to block all cookies in mail doesn't address webmail is no argument for not adding the feature. Netscape Mail is a mail client created by Netscape & Mozilla. Hotmail is a mail client created by Microsoft. We have a responsibility to protect users' privacy when they use the former, but not the latter. It's like saying "Outlook has this bug, so why should we bother to fix it in NS Mail?"
Assignee: naving → mstoltz
Target Milestone: --- → mozilla0.9.6
I can work on this bug but I am supposed to be concentrating on footprint issues (for Mach V) and emojo bugs atleast for now.
Navin, No hurry on this one, just keep it on the radar. We've had this problem forever; it can wait a few more months to get fixed. Retarget as necessary.
reassign to self, cc putterman for re-targetting.
Assignee: mstoltz → naving
Comments from "Paul Wagland" At the risk of starting of starting up another long running argument, I would just like to point out two things: 1) You do not have to set a cookie, image names are just as effective! I am not sure of the exact email format to set a cookie, but there is exactly no difference between: a) SET COOKIE harvestemail=12345 (img src="http://evil.dude.com/randomImage.jpg") and b) (img src="http://evil.dude.com/servletToHarvestEMail/12345/randomImage.jpg") Using both, I can match the image retrieval to the reading of the e-mail. The second might take an extra day's worth of work, but the result is exactly the same, and without using any cookies. 2) Images in HTML e-mail can be very useful from a trusted source. This issue is also addressed in BUG 28327, however the solution there is ensure that e-mail is multipart and include the images in the e-mail. This is fine technically, but rarely done in real life. In any event, while this is useful for the current spamming tricks, it will not be so useful for long since spammers will just learn to adjust, normally quicker than you would expect. Especially since it is so easy to do. This is actually one place where "zone based pages" ala IE do make a bit of sense, if you had one zone for internet, and one zone for mail, you (and netscape) could set quite reasonable defaults for mail (i.e. don't-accept-foreign-cookies and disable javascript et. al.) without hitting any problems on the "real" web.
We can already disable JS for mail while leaving it on in Navigator. I'm sure we could do the dame for cookies with minimal effort, although as you said, blocking cookies doesn't prevent these "web bugs."
> 1) You do not have to set a cookie, image names are just as effective! 1. Images might not be loaded at all, after bug 28327 is fixed. 2. Even if we load images (or other data) for a Mailnews message, that load should not be allowed to use a cookie in any way, neither set nor send.
moving to 0.9.8
Target Milestone: mozilla0.9.6 → mozilla0.9.8
Target Milestone: mozilla0.9.8 → mozilla0.9.9
Keywords: nsbeta1+
Priority: P3 → P2
Blocks: 31907
Target Milestone: mozilla0.9.9 → mozilla1.0
Keywords: mozilla1.0+
Keywords: mozilla1.0
QA Contact: lchiang → bsharma
Whiteboard: [ADT2]
After reading this lengthy bug report, I feel most people are for blocking all cookies in mail. I don't see how we can fix spammers confirming email address using image url. I think this issue can be addressed in a separate bug.
Yes, there are other bugs about adding options to show raw mail source rather than HTML, Show HTML rendered into plain text, and to block all web access when displaying mail (images, cookies, redirects, applets, plugins, etc). Ben Bucksch is working on some of these others, restricting this bug purely to cookies is perfectly appropriate.
As far cookies is concerned, this bug is worksforme/fixed because we already have support for blocking cookies not from originating website - foreign cookie. We have this pref in cookie panel Enable cookies for the originating web site only. So mail messages having img src as http urls will not be able to set cookies if this pref is set because documentURI (mail message uri) will be different from url (img src uri). you can look into cookie_isForeign() method for details. thanks morse for this point. I verified this in the debugger.
Status: NEW → RESOLVED
Closed: 25 years ago → 23 years ago
Resolution: --- → WORKSFORME
I disagree with comment 77. I do agree with comment 75: "After reading this lengthy bug report, I feel most people are for blocking all cookies in mail." This should be separate. If a user wants to block all cookies generated via e-mail and not block any cookies via the browser he/she should be able to do that without constantly changing preferences. I strongly support an option that allows a user to disable ALL cookies when viewing mail and having that option independent from the browser cookie settings.
navin, that pref is related, but does not cover this bug. Here's the issue Let's say someone send out an HTML e-mail with the following html snippet: <img src="http://www.mydomain.com/banner.jpg"> what happens is that HTTP will go and fetch this .jpg from www.mydomain.com. During this HTTP request, the server may request cookies from mydomain.com or some other domain like yahoo.com The pref prevents that HTTP request from requesting a cookie from yahoo.com, but it does not prevent that site from setting a cookie from mydomain.com this is why we need all cookies blocked from mail
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
All domains will be blocked from setting cookies (using foreign cookie pref) as long as it does not match your mail server domain, which is very unlikely that spammers can do.
s/domain/host
navin, that is not true. re-read my description of what happens, above. the HTTP request goes out to the HTTP server, not to your mail server. The cookie restrictions are on the HTTP request (being the image) not the originating document (being the mail message) This is as far as I know, hopefully mitch can confirm this.. so it doesn't matter what your mail server's hostname is, the cookie can still be set. maybe we're talking about different prefs? I'm talking about network.cookies.cookieBehavior, what pref are you referring to?
ya, I know the set cookie header is in http response. If you look in this method COOKIE_SetCookieStringFromHttp in nsCookies.cpp, it has a firstURL and curURL. So curURL is the http image url something like "http://www.mydomain.com/banner.jpg" and first url will be something like imap://naving@nsmail-2:143/Inbox/msg-id1, so this code will block cookie from being set. /* check for foreign cookie if pref says to reject such */ if ((cookie_GetBehaviorPref() == PERMISSION_DontAcceptForeign) && cookie_isForeign(curURL, firstURL, ioService)) { /* it's a foreign cookie so don't set the cookie */ return; } I'm also referring to the same pref network.cookie.cookieBehavior
doh! my bad. you're right.
Status: REOPENED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → WORKSFORME
Note that 3rd-party cookie-blocking was broken for a good long while, which is why even people with that pref set were seeing cookies set in mail. I think it's working again now.
I am frustrated that this has been marked as "resolved -- worksforme". Programmers may differ, but users consider reading email to be different from web browsing. I will tolerate web sites setting cross-domain cookies, but will not tolerate email setting cookies. The decision to allow different JavaScript permission between websites and email was exactly the right decision, and that decision should be made again for cookie acceptance. (I fully agree with comment 21 .)
reopen bug; reset milestone, etc. if necessary. I agree with comment 21 and comment 86 (and possibly others). The workaround of setting the browser cookie preference should not be considered the FIX for this bug. It's a workaround. A real fix would involve a separate setting. It is reasonable for me to want different settings for viewing e-mail and viewing a web page.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
So you want foreign/third party cookies to be set while browsing but not while reading mail. It doesn't seem very logical to me.
Some sites have image links that do not work if cookies can't be set. If you're voluntarily browsing such a site you'd want 3rd party cookies to work. That is a completely separate decision from whether you want cookies from crap that has involuntarily come into your mailbox and is inadvertently opened while you're trying to delete it.
All the comments that I'm reading in this bug report are about people not wanting cookies in their mail messages but not really giving any good reasons why. You all seem to have a gut feeling that mail-cookies are worse than website cookies but aren't able to justify that feeling. The reason is because of web bugs. When you visit a website, the site has no idea of your e-mail address and can in no way tag you with such identifiable information. When you receive an e-mail, the spammer knows your e-mail (he just wrote to you, didn't he) and so can create a cookie that contains it. Then if you happen to latter be surfing the web and come across a site that contains an image from the spammer (perhaps the spammer was doubleclick), suddenly the site knows all about you right down to your e-mail address.
In fact, it looks like comments 8 and 9 are saying exactly what I just said. But the latter comments are all being vague about the reason they don't want e-mail cookies.
We can revisit separate prefs later.
Target Milestone: mozilla1.0 → mozilla1.2alpha
Keywords: nsbeta1+ → nsbeta1-
Whiteboard: [ADT2]
ok, let us have an additional pref - "Disable cookies in Mail & Newsgroups" Right now we have Disable cookies Enable cookie for the originating web site only Enable all cookies We will add a separate checkbox item Disable cookies in Mail & Newsgroups This pref will be enabled as long as we don't have Disable cookies selected.
Keywords: nsbeta1- → nsbeta1+
Whiteboard: [ADT2]
Target Milestone: mozilla1.2alpha → mozilla1.0
Restoring putterman changes.
Keywords: nsbeta1+ → nsbeta1-
Whiteboard: [ADT2]
Oh the irony. http://news.com.com/2100-1023-875992.html this is a good reason to make an explicit pref. people can read articles like this and then go make sure "Block cookies in e-mail" is checked. Now, to answer the question as to why cookies are important in e-mail, the reason is this: Most people may not realize that their e-mail habits are being tracked just by reading e-mail. (Obviously the above article will educate some of those people) The previous assumption is that if you are surfing, they you are taking the risk that you may be tracked. The risk you are taking is that you are being tracked by the sites you visit, in some form or another. However, if you are trying to read your e-mail, you don't always know what site is tracking you, and you don't find out until the message has been displayed, and then it is too late. Spammers are essentially forcing you to be tracked by them, without your consent. Now, this also brings up the question of why cookies are different then someone just tracking your e-mail reading habits with special image urls or IFRAMEs.. I think the difference there is that cookies are persistent across messages and accounts. A spammer could spam someone with multiple messages, and watch to see how they react to different spam. For instance, if a user starts to get familiar with one particular spammer and adapts by deleting that spammer's messages, the spammer can discover this by the fact that cookies are no longer being set, and counter this adaption by changing the format of the subject/sender such that the user no longer recognizes the messages. In addition, now that we have multiple accounts support, a spammer can discover that a user has two e-mail addresses because a cookie set by one message on one account can be read by the spammer when another e-mail is read on another account. This is a serious invasion of privacy, for a company to learn that two e-mail accounts are connected to the same person. Now, as for the existing pref: We default to accepting all cookies, and not to the whole "originating site" behavior, for compatibility reasons. I think that we should have the opposite default for e-mail, which is exactly the pref which navin described above. We should default to NOT accepting cookies in e-mail. I'm renominating for nsbeta1 in light of recent press around this issue, with all the above argument explaining why I think its worthy of being reevaluated.
Keywords: nsbeta1- → nsbeta1
you've convinced me to keep it around.
Keywords: nsbeta1 → nsbeta1+
Whiteboard: [adt2]
You've convinced me too. I'll have a patch for it shortly.
Assignee: naving → morse
Status: REOPENED → NEW
Morse, I have already got the UI pref |cookies patch almost done. I will complete it and you can review it
Assignee: morse → naving
Attached patch proposed fix (obsolete) — — Splinter Review
The fix is to add an additonal pref for mailCookieBehavior. The default value of this pref prevents setting of cookies, if it comes from mail/news message. Also this pref is an overlay from mailnews just like we have for javascript. morse, can you review the cookie part of the patch.
1. Why do you have a larger vertical space between the added checkbox (in the cookie pref panel) than between the two already existing checkboxes? Did you show this to the UE group? 2. Please use a more descriptive name for the pref. From the name cookie_behaviorPrefForMailNews I don't know whether true means you will accept mailnews cookies or if it means that you won't accept mailnews cookies. With the above changes, r=morse
This pref is kind of separate from those radio button so I inserted a separator. Who does UE for navigatior | prefs panel ? jennifer, do you like this screenshot ? I will change the pref name to "network.cookie.disableCookieForMailNews"
1) I agree with morse. Is this the desired UI? Do we want any text above the checkbox? Run this by marlon and jglick. Also, what does it mean to have "Disable Cookies" set, but "Disable Cookies in Mail & News" unset? Is there (or should there be) any relationship between this new pref and the existing cookie prefs? And if so, should the UI reflect that? (Should this checkbox be disabled based on the other UI elements?) 2) I agree with morse, the names of the pref and the methods are wacky. cookie_GetBehaviorPrefForMailNews() cookie_behaviorForMailNews network.cookie.cookieBehaviorForMailNews
Comment on attachment 77971 [details] [diff] [review] proposed fix overall, this look good. a few comments: > > PRBool >+cookie_isFromMailNews(char *firstURL, nsIIOService* ioService) { >+ >+ if (!firstURL) >+ return PR_FALSE; >+ >+ NS_ASSERTION(ioService, "IOService not available"); >+ if (!ioService) >+ return PR_TRUE; // we cannot check the scheme of orignal uri do you really want to return PR_TRUE here? seems like we should default to PR_FALSE >+ >+ nsCAutoString schemeString; >+ nsCAutoString firstURLString; >+ firstURLString.Assign(firstURL); >+ nsresult rv = ioService->ExtractScheme(firstURLString, schemeString); Almost. See http://www.mozilla.org/projects/xpcom/string-quickref.html, the last section on "Converting literal strings to string objects" you should be using nsDependentString() >+ if (NS_FAILED(rv)) >+ return PR_TRUE; //malformed uri- better not to set the cookie >+ again, you think this is right? It seems like we should leave malformed URI handling to elsewhere in the cookie code - better not to be setting general policy in this function seems like PR_FALSE makes more sense. (i.e. if it's malformed, its not a mailnews URI) >+ const char* scheme = schemeString.get(); >+ return (!strcmp(scheme, "imap") || !strcmp(scheme, "news") || !strcmp(scheme, "mailbox")); use schemeString.Equals(NS_LITERAL_CSTRING("imap")) || .... it is faster than strcmp >+ >+ /* check if the cookie is being set from a MailNews message */ >+ if (cookie_GetBehaviorPrefForMailNews() && cookie_isFromMailNews(firstURL, ioService)) >+ return; > make sure to change the function names in addition to just the pref name. i.e. cookie_GetBehaviorPrefForMailNews should be cookie_GetDisabledForMailNews or something Nice job using a dynamic overlay though!
Comment on attachment 77971 [details] [diff] [review] proposed fix overall, this look good. a few comments: > > PRBool >+cookie_isFromMailNews(char *firstURL, nsIIOService* ioService) { >+ >+ if (!firstURL) >+ return PR_FALSE; >+ >+ NS_ASSERTION(ioService, "IOService not available"); >+ if (!ioService) >+ return PR_TRUE; // we cannot check the scheme of orignal uri do you really want to return PR_TRUE here? seems like we should default to PR_FALSE >+ >+ nsCAutoString schemeString; >+ nsCAutoString firstURLString; >+ firstURLString.Assign(firstURL); >+ nsresult rv = ioService->ExtractScheme(firstURLString, schemeString); Almost. See http://www.mozilla.org/projects/xpcom/string-quickref.html, the last section on "Converting literal strings to string objects" you should be using nsDependentString() >+ if (NS_FAILED(rv)) >+ return PR_TRUE; //malformed uri- better not to set the cookie >+ again, you think this is right? It seems like we should leave malformed URI handling to elsewhere in the cookie code - better not to be setting general policy in this function seems like PR_FALSE makes more sense. (i.e. if it's malformed, its not a mailnews URI) >+ const char* scheme = schemeString.get(); >+ return (!strcmp(scheme, "imap") || !strcmp(scheme, "news") || !strcmp(scheme, "mailbox")); use schemeString.Equals(NS_LITERAL_CSTRING("imap")) || .... it is faster than strcmp >+ >+ /* check if the cookie is being set from a MailNews message */ >+ if (cookie_GetBehaviorPrefForMailNews() && cookie_isFromMailNews(firstURL, ioService)) >+ return; > make sure to change the function names in addition to just the pref name. i.e. cookie_GetBehaviorPrefForMailNews should be cookie_GetDisabledForMailNews or something Nice job using a dynamic overlay though!
Attachment #77971 - Flags: needs-work+
use schemeString.Equals(NS_LITERAL_CSTRING("imap")) || .... it is faster than strcmp How is equals faster than strcmp, Can you explain ?
Hi Navin, as Seth mentions, is there any connection between the existing cookie radio buttons and this new pref? Are the radio items only for browser and this new checkbox is only for Mail/News?
The radio button determines whether this checkbox is enabled/disabled. If we have selected Disable all cookies, this checkbox will be disabled otherwise enabled. So the radio buttons affect the overall cookie behavior (MailNews + Navigator)
Attached image Example1 —
Is this appropriate?
Attached image Example2 —
from jglick's attachment: What does cookie-from-originating-server mean in the context of mailnews?
Those three options do not apply to mail and newsgroups. For mail/news you have either disable/enable cookies.
Attached image Example3 —
Thanks for the clarification. How is this? The attachment in #100 seems a little confusing because normally a checkbox is not enabled/disabled based on the setting of a separate group of radio buttons.
In example3 a user can disable navigator cookies and have mailnews cookies enabled, which is not a likely scenario. We want to avoid this type of selection. morse, alecf what do you think ?
Hmm, OK. Trying to think of a way to do this so we don't have a checkbox that can be enabled/disabled based on the setting of a group of separate radio buttons, which is a little confusing. I'll keep thinking about it. Marlon, any suggestions?
I had the identical thought but hesitated to express it in my earlier posting. Basically we are trading a 4-line UI (3 radio buttons, one checkbox) for another four-line UI (2 checkboxes, 2 radio buttons) for the advantage of having more versatility which the user really doesn't want (enabling mailnews cookies but not web cookies). And the downside is that we are forcing the users who are familiar with the old UI to learn a new one.
Don't forget the p3p choice in the set of radio buttons. You have been leaving them out in the screenshots you've been posting.
i have an idea - i think we should use reverse terminology to proactively keep users from doing the wrong thing. if we word the pref such that it implies this feature is protecting them and that they don't need to re-enable it (in hopes of access a disabled feature for example) - make it a checked check box, and reverse the meaning. this prevents users who casually browse their prefs turning things on which and might fall into the 'if it sounds good turn it on' trap.
>Don't forget the p3p choice I started from a screenshot of today's build. I didn't see any p3p choices. As I mentioned before, having a checkbox enabled/disabled based on a separate set of radio buttons might confuse some folks, but I can't think of another ui at the moment that would work for the behavior you want. So I suppose we go with Navin's screenshot unless we can think of something better for now.
> I didn't see any p3p choices That's because you didn't build the p3p directory
Re Marlon's suggestion: (disabling prevents spammers from validating your e-mail) That's not true. The simplest way to validate your e-mail is to put an image in the e-mail and then wait for your browser to fetch the image. That validates your e-mail address AND tells them the exact time at which you read the message. So your parenthetical remark will give users a false sense of security.
while that's true, within the context of cookie management the explanation i propose is not negated by another method such as imagesrc loading. it only claims to addresses the cookie method, but by no means every method of validation (hence the parenthesis).
my 2 cents: I like the first screenshot from Navin best. It should be clear that of "Disable cookies" is chosen, you cannot enable cookies in Mailnews, esp. when the checkbox is disabled then. I don't think, that this will confuse anybody, considering the texts/subject. You have a point that the arrangement of the UI doesn't make this connection clear. However, there are tons of places in our pref UI (incl. Mailnews) where this connection is similarily unclear, and fixes have been rejected. Otherwise, I like "Example 3" best. > (disabling prevents spammers from validating your e-mail) I agree with morse. While the intend is certainly good (explains what this obscure pref is good for), the wording is wrong. You don't limit your claim to cookies, so, judging from your statement, the user will assume, that cookies are the only option to track and that he is safe, when this is disabled. Which is completely wrong. This leads to the question: Why on earth would a user want to enable this? IMO, just turn cookies for Mailnews off, without UI to turn them on again.
> This leads to the question: Why on earth would a user want to enable this? IMO, > just turn cookies for Mailnews off, without UI to turn them on again. yes, yes, yes! The additional UI is unnecessary and misleading. I can't imagine any useful use of setting the cookies in mail messages. At most there could be release note that it is possible to allow them by setting the pref manually in prefs.js. So no messing with UI and no need to resolve the complicated UI issue.
We are disabling other checkboxes below this new one I added, based on radio button selection (comment #100) so I think it should be ok. I will attach a new patch.
> We are disabling other checkboxes below this new one I added, based on radio > button selection We certainly are. The warn-before-accepting-cookies checkbox is an example. So I don't understand why there is so much concern about naving's proposed design, other than a few esthetic issues like the excessive space (see comment 101).
Attached patch proposed fix, v2 — — Splinter Review
Same fix as last one, changes done as per reviewers. screenshot remains the same as in comment #100. I also build p3p and it looks ok w/ p3p also. I will attach one more screenshot in modern. looking for r & sr
Attachment #77971 - Attachment is obsolete: true
Maybe eliminate the space between the "Mail & Newsgroup" checkbox and the other checkboxes below it since they all are enabled/disabled based on which cookie radio button is selected?
I agree about the space, as I mentioned that in comment 101. With that fix, r=morse
Comment on attachment 78375 [details] [diff] [review] proposed fix, v2 sr=dveditz When you mail drivers be sure to mention you're changing localizable strings
Attachment #78375 - Flags: superreview+
Attachment #78375 - Flags: review+
r/sr=sspitzer on the mailnews parts.
Keywords: adt1.0.0
CCing Michele and Tao for l10n and preferences changes
Hi, Naving: would you please explain what the new pref does in 121324? thx!
Blocks: 121324
Comment on attachment 78375 [details] [diff] [review] proposed fix, v2 just to add, the reason that foo.Equals(NS_LITERAL_CSTRING("imap")) is faster is that NS_LITERAL_CSTRING includes the length of the string at compile time when making the temporary, so that nsAString::Equals() can first compare length, and see if schemeString.Length() == temporary.Length(), which is faster than strcmp()
Comment on attachment 78375 [details] [diff] [review] proposed fix, v2 In this patch I have made network.cookie.disableCookieForMailNews to be true by default. I propose changing it to false because only advanced users who know more about cookies can go and set it to true and at the same time it will not affect majority, who I believe don't care about it
Comment on attachment 78375 [details] [diff] [review] proposed fix, v2 a=asa (on behalf of drivers) for checkin to the 1.0 branch.
Attachment #78375 - Flags: approval+
> In this patch I have made network.cookie.disableCookieForMailNews to be true > by default. I propose changing it to false because only advanced users who > know more about cookies can go and set it to true and at the same time it > will not affect majority, who I believe don't care about it But that's exactly why you would want to leave it as true. If everything said in this bug report is correct, there is no valid reason for ever having cookies enabled in mailnews and the *only* purpose it can ever serve is for spammer tracking. If that's true, then keep mail cookies disabled by default. But make sure that there isn't some use that we are overlooking which will cause some functionality to be broken if we have it disabled.
I strongly agree with Mr. Morse. Providing strong security and privacy settings by default will cause more people to use Mozilla and Mozilla-based products. Some potential restrictions are controversial than others, but this one seems to have widespread support.
I'm not sure how others feel but I don't think it's acceptable to make a change to a patch that already has reviews and super-reviews and approval. Navin said: >In this patch I have >made network.cookie.disableCookieForMailNews to be >true by default. I propose >changing it to false because only advanced users who know more about cookies >can go >and set it to true >and at the same time >it will not affect majority, who I believe don't care about it If there is a real desire to change this preference, I would like to see a new patch go through all of the process again since the change in default is not what was approved or discussed in this bug. Certainly the mozilla build would prefer the patch as originally reviewed/approved rather than changing the default preference.
If I change the patch, I will seek r/sr/a again.
you only need a minor tweak to the patch (one line) so you can post that as a seperate patch, and you'll get r/sr/a much faster. I also agree that cookies should be blocked in mail by default.
Pls land this on the trunk to bake for a few days, have QA look, and make sure there are no regressions introduced, while ADT discusses approval for this fix. Removing adt1.0.0. Pls renominate once it has been on the trunk a couple days, and been tested.
my point was just that it makes no sense to request review of a 100+ line patch if the change is discussed and the actual change in only one line. Why make it more difficult for everyone by burying a one-line change in a big patch? The patch can be annotated appropriatey "change the default pref from the previous patch"
proposed fix, v2 checked on trunk. leaving open for adt branch consideration.
Keywords: adt1.0.0
Testcase URL: http://www.mozilla.org/quality/mailnews/tests/sea-mn-cookies.html Platforms / Builds: Mandrake Linux 8.1 - 2002-04-15-10 Mac OS X 10.1.3 - 2002-04-15-08 Windows 2000 - 2002-04-15-03 Mac OS 9.2.2 - 2002-04-13-08 Protocols tested: AOL Mail, Netscape WebMail, POP3, IMAP, NNTP I'm sure my testcase could be expanded (this is my first excursion into such a feature), but I've covered a good majority of the preferences, migration scenarios, etc. Thanks to Tom Everingham (tever@netscape.com) for the gracious, simple test Perl script. Verified FIXED on the trunk - leaving bug resolution 'as-is' until this lands on the branch and can be verified there. ADT - this is a good feature and I feel it should be enabled on the branch.
QA Contact: bsharma → stephend
Whiteboard: [adt2] → [adt2] [verified on trunk]
marking fixed, we use keywords for trunk.
Status: NEW → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
we use keywords for branch
Keywords: adt1.0.0
adding adt1.0.0+. Please check in as soon as possible and mark this fixed1.0.0 when you do.
Keywords: adt1.0.0 → adt1.0.0+
CCing cotter and foster-clark for assessing doc impact.
Navin, i tried to edit the string "Disable cookies etc ..." in mailPrefoverlay.dtd file and all changes to this string where i replace the first word with accented ascii or ja chars doesn't work.After i restart with a new edited file I get the string in red ( which means that it couldn't be found?) in the cookies tab.Nothing like that happens if i add or replace with ascii chars. I am cc'ing to Ray Chen, he might add his comments here. The build i used for those manipulations was the trunk build 04-15.
marina, I tried it on my machine and it displayed JA character just fine. I can forward the file to you if you want.
Verified per my above comment 146 on trunk builds.
Status: RESOLVED → VERIFIED
This was checked into branch yesterday.
Keywords: fixed1.0.0
Using 1.0.0 BRANCH builds: Mac OS 9.2.2 - 2002-04-22-05 Mac OS X 10.1.4 - 2002-04-22-05 Mandrake 8.1 - 2002-04-22-08 Windows 2000 - 2002-04-22-06 and the testcase at: http://www.mozilla.org/quality/mailnews/tests/sea-mn-cookies.html this is VERIFIED FIXED. Caveat: I am seeing some _very_ strange chrome bugs on Mac OS 9.2.2 and Mac OS X 10.1.3 with the stub installer and the .sit for OS X. I'll dig through Bugzilla and see what I can find. I DO NOT see this on Windows 2000 or Linux. Basically, here's what I'm seeing: When I upgrade from trunk/6.2.2/other branch build without the XUL fix, it works fine. However, upon reboot, the menuitem 'Disable cookies in Mail & Newsgroups' simply *vanishes*! I'm not sure if this was related to Hewitt's massive <outliner> to <tree> conversion, there is some kind of rendering error, or if we have some element being read as hidden="true", or what. It's very strange. I did all my verifications while the panel's menuitem was still available, (since the pref is enabled/disabled in realtime) and I've verified the fix. Replacing fixed1.0.0 with branch1.0.0...
Keywords: fixed1.0.0 → verified1.0.0
See bug 140149 for the bug I filed on the Mac's weird 'disappearing pref UI' behavior.
Mass removing self from CC list.
Now I feel sumb because I have to add back. Sorry for the spam.
*** Bug 168258 has been marked as a duplicate of this bug. ***
Reopening per the new information reported in bug 168258. It turns out that the patch checked in for this bug does not prevent .css files referenced in mail messages from setting cookies.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Problem is that the following test in nsCookies.cpp is not catching the fact that the cookie is being set during the reading of a mail message. The scheme for firstURL is coming up as "http". PRBool cookie_isFromMailNews(nsIURI *firstURL) { if (!firstURL) return PR_FALSE; nsCAutoString schemeString; nsresult rv = firstURL->GetScheme(schemeString); if (NS_FAILED(rv)) //malformed uri return PR_FALSE; return (schemeString.Equals(NS_LITERAL_CSTRING("imap")) || schemeString.Equals(NS_LITERAL_CSTRING("news")) || schemeString.Equals(NS_LITERAL_CSTRING("snews")) || schemeString.Equals(NS_LITERAL_CSTRING("mailbox"))); }
*** Bug 168174 has been marked as a duplicate of this bug. ***
Ignore comment above. I didn't mean to mark bug 168174 as a dup of this one.
Blocks: 160315
re-assigning nsbeta1+ bugs
Assignee: naving → sspitzer
Status: REOPENED → NEW
hope to get to this for 1.3 final.
Status: NEW → ASSIGNED
Target Milestone: mozilla1.0 → mozilla1.3final
Whiteboard: [adt2] [verified on trunk] → [adt2] [not blocking css files]
Flags: blocking1.3? → blocking1.3-
Target Milestone: mozilla1.3final → mozilla1.4beta
privacy, over to cavin.
Assignee: sspitzer → cavin
Status: ASSIGNED → NEW
Seth: Whenever I try to send you a mail I get: sspitzer@netscape.com SMTP error from remote mailer after MAIL FROM:<browserspy-htmltest-reply@gemal.dk>: host ywing.aoltw.net [204.29.187.151]: 550 5.7.1 Access denied. IP name lookup failed [194.192.187.65] that's why you didn't get the HTML test mail from BrowserSpy
Flags: blocking1.4b? → blocking1.4b+
cavin, this might be a dup of darin's bug, bug #180983 here's some info on how to fix this. the remote CSS is loaded by a http channel someone is just not setting the originating URL property of nsIHttpChannelInternal probably the CSS loader needs to do that... see nsCSSLoader.cpp:1374 we need to QI httpChannel to nsIHttpChannelInternal and call SetDocumentURI we'd need to know the URI of the toplevel mail message cookies uses the http channel's documentURI property to determine the original URL loaded by the user so we have to pass that attribute down to all the channels created which is an extremely fragile system see nsDocShell.cpp:5316 there you can see SetDocumentURI being called for a new load in a docshell so if you could get to the toplevel document from the CSS loader then you could pass the toplevel document's URL to nsIHttpChannelInternal::SetDocumentURI
*** Bug 180983 has been marked as a duplicate of this bug. ***
Perhaps we just want an accessor on the document that people could call to get this toplevel URI? The document could ask the docshell or something.... I'd prefer it to happen automagically so we don't have yet another piece of code that all necko consumers have to have, but....
Over to Darin as he's looking into the issues.
Assignee: cavin → darin
Attached patch v0.1 patch (obsolete) — — Splinter Review
so, this patch is just a prototype... it adds some ugly code to cookies which leverages the fact that the channel's notification callbacks is implemented by the docshell. this allows cookies to check whether or not the channel is associated with a mailnews docshell. this is pretty bulletproof and doesn't require any modifications to necko consumers (so long as they setup the notification callbacks "correctly"). this is of course extremely ugly since 1) cookies shouldn't depend on docshell and 2) cookies shouldn't know that docshell is the implementation of necko's notification callbacks. i'm only considering this patch because of how simple it is and because of the fact that 1.4 beta is nearly here and gone. ultimately, i think we'd want cookies to talk to some intermediate/more-generic interface, which docshell could implement. long term querying the channel's notification callbacks for the required bit of information seems a lot better than checking URI schemes. seems to me that docshell should observe the "no cookies in mailnews" pref and the more generic interface should really just be something which answers the question "are cookies allowed?" there is one big problem with this patch: it only prevents SPAM from setting cookies. it doesn't prevent SPAM from reading cookies. to fix that i need to rev nsICookieService and the "GetCookie" methods to pass a nsIChannel. also, because nsICookieService::SetCookieString takes a nsIHttpChannel, this patch doesn't prevent a <meta HTTP-EQUIV="Set-Cookie" ...> tag from setting a cookie when it appears in a document served up via imap://. so, i think i need to fix those other cases before finalizing this patch for 1.4 beta. i'm probably going to switch nsICookieService over to passing nsIChannel and perform the necessary QI's inside the cookies code (P3P only wants to talk to nsIHttpChannel).
Attached patch v1 patch — — Splinter Review
ok, this version includes the changes to nsICookieService i mentioned above. i haven't thoroughly tested it yet, but it does seem to prevent cookies from being sent as well as set by content loaded in a mailnews docshell. it should take care of the <meta> tag version as well, but i haven't verified that yet.
Attachment #122123 - Attachment is obsolete: true
Attachment #122131 - Flags: review?(bzbarsky)
*** Bug 160315 has been marked as a duplicate of this bug. ***
Attachment #122131 - Flags: superreview?(dveditz)
Comment on attachment 122131 [details] [diff] [review] v1 patch dwitte: would make good sense for you to review this patch. thx!
Attachment #122131 - Flags: review?(bz-bugspam) → review?(dwitte)
>+ } while (appType != nsIDocShell::APP_TYPE_MAIL && >+ NS_SUCCEEDED(item->GetParent(getter_AddRefs(parent))) && >+ parent); is the |parent| check redundant in favor of the NS_SUCCEEDED? >+ if ((appType == nsIDocShell::APP_TYPE_MAIL) || >+ (aFirstURI && cookie_IsFromMailNews(firstURIScheme)) || >+ cookie_IsFromMailNews(currentURIScheme)) { should we be mangling these checks together? is the former inclusive of the URI checks? do we need one or both of the URI checks anymore? would it be worth separating out these checks a bit? i'm not sure how expensive the docshell-walking is, but perhaps it'd be worth checking the URI's first, and if they don't tell you to block, then go walking. i have no idea though, up to you. ;) >- nsCookieStatus p3pStatus = cookie_P3PDecision(aHostURI, aFirstURI, aHttpChannel); >+ nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(aChannel); >+ nsCookieStatus p3pStatus = cookie_P3PDecision(aHostURI, aFirstURI, httpChannel); >+ nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(aChannel); // get the site's p3p policy now (common to all cookies) >- nsCookiePolicy cookiePolicy = cookie_GetPolicy(P3P_SitePolicy(aHostURI, aHttpChannel)); >+ nsCookiePolicy cookiePolicy = cookie_GetPolicy(P3P_SitePolicy(aHostURI, httpChannel)); these two are very similar... if you think it's worth it, perhaps you could just shift the QI into P3P_SitePolicy. >-#include "nsIURI.idl" >-#include "nsIPrompt.idl" >-interface nsIHttpChannel; >+interface nsIURI; >+interface nsIPrompt; >+interface nsIChannel; nice ;) this looks great, although i obviously can't speak for the nsHTMLDocument fu (will bz review that?). also, just a side note, nsImgManager is doing some fairly similar stuff here: http://lxr.mozilla.org/seamonkey/source/extensions/cookie/nsImgManager.cpp#121 perhaps at some point we want to consolidate these two mailnews-checking portions, but that can wait til later. thanks for doing this!
Attachment #122131 - Flags: review?(dwitte) → review+
> is the |parent| check redundant in favor of the NS_SUCCEEDED? nsDocShell::GetParent may return NS_OK and a null result. in fact, i think i recall hitting a crash without the null check on parent. > would it be worth separating out these checks a bit? i'm not sure how > expensive the docshell-walking is, but perhaps it'd be worth checking the > URI's first, and if they don't tell you to block, then go walking. i have no > idea though, up to you. ;) i thought about that too... and maybe for performance reasons it'd be a good idea, but it would only improve the case in which we were actually in mailnews with cookies blocked. also, originally, i thought of the docshell walking as the preferred mechanism. in fact, i was thinking of doing away with the URL-scheme checking altogether since it is not as scalable and basically redundant. hmm... maybe it would be best to flip the order. > these two are very similar... if you think it's worth it, perhaps you could > just shift the QI into P3P_SitePolicy. good catch. thx! yeah, the image manager stuff caught my attention too. thanks for the code review. i'll update the patch... hopefully bz will have a chance to review this too :)
Comment on attachment 122131 [details] [diff] [review] v1 patch sr=dveditz with two caveats: - need an r= from layout folks, should just be able to glance at it - are embeddors using nsICookieService? I know there are "unofficially frozen-like" interfaces lurking around, please check with jjimata.
Attachment #122131 - Flags: superreview?(dveditz) → superreview+
Comment on attachment 122131 [details] [diff] [review] v1 patch seeking drivers approval for this security/privacy bug fix.
Attachment #122131 - Flags: approval1.4b?
Comment on attachment 122131 [details] [diff] [review] v1 patch a=sspitzer, since we want this privacy bug fixed. darin, can you make sure that QA knows about this change, and where to look for potential regressions?
Attachment #122131 - Flags: approval1.4b? → approval1.4b+
Comment on attachment 122131 [details] [diff] [review] v1 patch Is it just me or do the javadoc comments in nsICookieService.idl have no bearing on reality? What's aCookie in the comments? What's the aChannel callers are supposed to pass in? Is passing null valid? Is there a good reason to store both mChannel and mHttpChannel in nsHTMLDocument?
bz: yeah, nsICookieService.idl documentation is a complete mess (it's on my list of interfaces to cleanup... maybe i'll get to it for this patch). as for storing mChannel and mHttpChannel, i thought about just storing mChannel w/ a QI to nsIHttpChannel when needed, but went with this solution since we probably don't care about the extra (4 bytes of) memory used by the document. do you have a preference?
fixed on trunk, plus revised comments for nsICookieService.idl (i'm also going to file a bug to further cleanup nsICookieService... some of the methods seem to be completely redundant.)
marking FIXED (for real).
Status: NEW → RESOLVED
Closed: 23 years ago → 22 years ago
Resolution: --- → FIXED
Using build 20030523 on winxp, macosx and linux: The test site for cookies mentioned in the test case in comment 146 is not available so I have tested by doing a Send Page of several sites (Netscape home page, cbsnews, abcnews, askjeeves)opened some AOL alerts messages with the cookies options in the preferences set to test the various scenarios: Enable all cookies =ON Disable cookies in Mail&Newsgroups =OK Enable all cookies =ON Enable cookies in Mail&Newsgroups, no prompt =OK Enable all cookies =ON Enable cookies in Mail&Newsgroups, with prompt, Deny = OK Enable all cookies =ON Enable cookies in Mail&Newsgroups, with prompt, Deny& remember = OK Enable all cookies =ON Enable cookies in Mail&Newsgroups, with prompt, Allow = OK Enable all cookies =ON Enable cookies in Mail&Newsgroups, with prompt, Allow& remember = OK Performed the test in bug mentioned in comment 160. All passed. Verified. Note: if there are other tests that need to be performed for this bug please list them and give site locations that can be used to run the tests.
Status: RESOLVED → VERIFIED
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: