Closed Bug 22994 Opened 25 years ago Closed 21 years ago

Mail reader allows spammers to set cookies to track web usage

Categories

(MailNews Core :: Security, defect, P2)

Tracking

(Not tracked)

VERIFIED FIXED
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)

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 ago22 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: 22 years ago22 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: nsbeta1nsbeta1+
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, v2Splinter 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: 22 years ago22 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.0adt1.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...
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 patchSplinter 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: 22 years ago21 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: