87388 - third party cookie blocking doesn't work when "Enable cookies for the originating Web site only" is set

Status: VERIFIED FIXED

(Core :: Networking: Cookies, defect, P2)

Description

[J Luh]

Using 2001062209 Mac binary on Mac OS X (via Classic), Mozilla accepts
third-party cookies even if "Enable cookies for the originating Web site only"
is set.

To reproduce:

1) Create a new profile.
2) Edit->Preferences. Set cookie preferences to "Enable cookies for the
originating Web site only"

3) Visit http://www.nytimes.com/

4) Tasks->Privacy and Security->Cookie Manager->View Stored Cookies

5) Verify that you now have third-party cookies from bigcharts.com and
centrport.net.

The img tags are not enclosed within an iframe, so this is not the same as bug
67447.

Comment 1

[Stephen P. Morse]

Unfortunately many sites get around the third-party cookie blockage by doing 
redirects.  A redirect is not considered as a third party.  And we don't want to 
block cookies from redirected sites because that would have severe consequences.

I don't know if that is what is going on at the nytimes site, but it could very 
well be.  And in that case this bug report is invalid.

So I'm closing this out.  But if you can determine that this really is a 
third-party cookie and not a redirected cookie, then please reopen.

Comment 2

[J Luh]

I assume by redirects you mean that the HTML source points to an image that is
stored on a same-domain server, which redirects you to a third-party server when
you request the image. That does not appear to be the case here, so I am
reopening this bug.

View->Page Source shows that the HTML source of the page points to images hosted
on charts.bigcharts.com and mds.centrport.net. If you visit those foreign links
by themselves, the same cookies get set.

Comment 3

[Stephen P. Morse]

No, I wasn't referring to content on the page that is redirected.  I was 
referring to an entire page that is redirected.

For example, if your http request specifies that you want to go to 
www.firstsite.com and the http reply that you get back is a redirect to 
www.secondsite.com, then a redirect has occured.  This typically occurs when a 
website has moved.  And in that case you still want secondsite to be considered 
as the first party and not the third party.

But marketing sites have used this as a loophole to set third party cookies even 
though you are blocking such cookies.  Specifically they do the following:

1. You send out a request to visit www.nytimes.com
2. www.nytimes.com send back a redirect to www.doubleclick.com
3. doubleclick is now the first-party site and goes and sets a cookie
4. doubleclick knows that nytimes was the referer so it now does a redirect back 
to that site
5. now nytimes.com puts up its webpage.

The way to prove that this is what is happening is to use a sniffer.  But I'm 
almost certain that this is what is going on here because it is a standard trick 
that websites are now using to circumvent the block-third-party-cookie 
preference setting.

Comment 4

[J Luh]

I don't think these cookies are being set through redirects. I don't have a
sniffer, but "lynx -trace http://www.nytimes.com/" doesn't come up with any
redirects in the trace log.

There is evidence the cookies are NOT being set by redirects, and so far no one
has confirmed that this is being done with redirects. If you are not willing to
investigate, can you at least leave this bug open and "unconfirmed" so someone
else can investigate, instead of just declaring it "invalid" out of hand?

Comment 5

[J Luh]

Attachment: this page sets cookies

Comment 6

[J Luh]

Attachment: test case for setting third-party cookies

Comment 7

[J Luh]

On my setup (2001062209 on Mac), the stripped-down test case (second attachment)
still sets third-party cookies when served from bugzilla.mozilla.org.

Comment 8

[Stephen P. Morse]

This appears to be a networking problem.  I too have a simplified test case -- 
it is at

   http://people.netscape.com/morse/bug.htm

Unfortunately this won't be visible outside the netscape firewall until early 
tomorrow (Sunday) morning.  In the meantime Netscape employees can get to it 
from inside the firewall at

   http://peoplestage/morse/bug.htm

What is happening is that the bug.htm content contains an image tag for an image 
on mds.centrport.net.  The following traffic occurs when bug.htm is requested:

1. Request for bug.txt
2. Response contains content including image tag for mds.centrport.net
3. Request for file on mds.cenrport.net
4. Response contains set-cookie and a redirct to another file on same server

(The complete traffic report is attached.)

In step 4 the set-cookie is causing a cookie to be set.

The cookie-setting code is in the SetCookieStringFromHTTP routine in 
nsCookieService.cpp.  One of the parameters passed to that routine is aFirstURL.  
That should be the URL for bug.htm but instead is the URL for the file on 
mds.centrport.net.

So now we need to see where aFirstURL is being set.  That is occuring in the 
calling routine which is OnExamineResponse in the file nsCookieHTTPNotify.cpp.  
Specifically it happens on line 220 which reads:

   rv = aHttpChannel->GetURI(getter_AddRefs(pFirstURL))

So the GetURI is returning the wrong URL.  That is why the cookie module does 
not recognize this as a third-party cookie.

Reassinging to networking.

Comment 9

[Stephen P. Morse]

Attachment: captured traffic

Comment 10

[Darin Fisher]

this is not a networking bug!  GetURI returns the URI used to create the channel.
perhaps what you need is GetOriginalURI?!?

-> morse

Comment 11

[Stephen P. Morse]

When block-third-party-cookies was implemented, I was told by the networking 
people that the call I needed to make was GetURI and not GetOriginalURI.

If an access is made to site A and site A does a redirect to site B and site B 
attempts to set a cookie, will GetOriginalURI return A or B?  If it returns A, 
then it is the wrong thing for the cookie test to be using.

Comment 12

[Darin Fisher]

If an access is made to site A and site A does a redirect to site B and site B 
attempts to set a cookie, GetOriginalURI will return site A, GetURI will return
site B.

Comment 13

[Stephen P. Morse]

Then that's why I needed to use GetURI and not GetOriginalURI.

Now when access is made to site C and site C includes an image tag for site D, 
and site D attempts to set a cookie, what will the two routines return?  In this 
case I need to get back C and not D.

Comment 14

[J Luh]

As a user, I think of a third-party cookie as any cookie whose domain doesn't
match the URL currently being shown in the URL bar of the window. Does that
translate acceptably into the implementation, and if so, isn't there some call
that will return that URL?

Comment 15

[Darin Fisher]

morse:

when access is made to site C and site C includes an image tag for site D, 
and site D attempts to set a cookie, GetOriginalURI will return site D.
GetURI may return site D or some other site if the image tag results in a
redirect.

Comment 16

[Paul Chen]

nav triage team:

Marking nsbeta1+, nsBranch, and p2

Comment 17

[Stephen P. Morse]

Based on Darin's two responses, neither GetURI nor GetOriginalURI are 
satisfactory for determining if we have a third-party cookie.  Is there some 
other call that I can use, or can a new API be added to return what I need.

Basically I need the following:

case 1:

   If an access is made to site A and site A does a redirect to site B and
   site B attempts to set a cookie, I need to get site B.

case 2:

   If an access is made to site C and site C includes an image tag for site D,
   and site D attempts to set a cookie, I need to get site C.

In summary, I need the first site after the last redirect, or the original site 
if there were no redirects.

Comment 18

[Stephen P. Morse]

> As a user, I think of a third-party cookie as any cookie whose domain doesn't
> match the URL currently being shown in the URL bar of the window.

If that equates to the criteria that I just stated, that's fine.  If not, this 
request would be for a change in semantics over what 4.x (and current 6.x) and I 
would recommend agains making any such change at this late date.  We would have 
no idea as to what sites will be broken as a result of making such a change.

Comment 19

[Gagan]

steve what you need to do is to get the URI thru the default channel of 
the loadgroup (which is always the parent page) So in your example of a page C 
with an image on D. 

- From the channel on D get the loadgroup
- From that loadgroup get the default channel (which will be for C)
- From that channel get the URI (getOriginalURI if you need the one that it was 
initiated with and getURI if you need the one it resolved to after redirects)

Comment 20

[Darin Fisher]

morse:

it sounds like what you need is to be able to get the URL for "the final"
toplevel document always.  is that right?  unfortunately, i'm not sure there is
a way to
always do that given a nsIHttpChannel.  you can check to see if the http channel
is the toplevel page by checking for the LOAD_DOCUMENT_URI load flag on the
channel (see nsIRequest::GetLoadFlags), and you can get the nsILoadGroup
associated with the channel, but there is no requirement that a channel have a
load group (and imagelib doesn't associate a load group with the http channels it
creates).  so, i'm sort of at an end here... i'm not sure what to do.

Comment 21

[Darin Fisher]

gagan: that won't always work because imagelib doesn't associate load groups w/
the http channels it creates.

Comment 22

[Gagan]

doh! You are right about the images case darin. Your suggestion would help in 
knowing whether the image/channel is top level or not but I think morse still 
needs a way to get to the top level document (uri really). I think this can be 
safely solved if imagelib sets the toplevel document's URI on to the dummy-load 
group that it creates (or read alternatively as-- set the default load channel 
to be the same as the original loadgroup's) That way my suggestion would still 
work. Pav do you think this might affect your stuff? What is the default channel 
in imagelib's dummy load group? 

Comment 23

[J Luh]

The logic sounds right as described by [e-mail], but 
frames and iframes (see bug 67447) also need to be handled somehow. 
I personally don't think iframes should be able to set cookies, but it 
doesn't make a big difference to me and I don't have an opinion at all on 
frames. Also other forms of non-page, non-image content like stylesheets 
and external .js

Comment 24

[Darin Fisher]

as far as these other elements are concerned, let's just make sure we are
compatible with nav4x and IE.

Comment 25

[Stephen P. Morse]

We are not compatible with nav 4.

1. I set my prefs to no-third-party cookies, and warn before accepting cookies.  
2. I go to my test site http://people.netscape.com/morse/bug.htm

nav 4 results: no warning, no cookie is set
nav 6 results: warning appears

I don't see any way to block 3rd-party cookies in my version (5.50) of IE so I 
don't know if we are compatible with them or not.

Comment 26

[Stephen P. Morse]

Based on the comments from darin above, it appears that I can't solve the 
problem unless networking gives me an API that returns the URL for "the final"
toplevel document always.  I've already tried assigning this to networking but 
it was bounced back to me.  If it remains my bug, I'll have to close it out as 
wont-fix, which would be unfotunate because we are then no longer compatible 
with our previous browsers.  darin, please let me know if I should do that or if 
you will accept the assignment of this bug.

Comment 27

[Gagan]

morse: sorry to burst your bubble but the bug is still yours... you are only 
blocked on doing this till bug 87388 gets resolved. In the meantime you could go 
ahead and implement my suggestion-- that will take care of most of the cases 
for you. When bug 87388 is fixed this will work correctly for all the remaining 
imagelib cases as well.

Comment 28

[Stephen P. Morse]

gagan, sorry to burst *your* bubble, but what you suggested I do is exactly what 
the present state of affairs are.  Namely

   - From the channel on D get the loadgroup
   - From that loadgroup get the default channel (which will be for C)
   - From that channel get the URI (getOriginalURI if you need the one that it
     was initiated with and getURI if you need the one it resolved to after
     redirects)

The code that is currently in nsCookieHTTPNotify.cpp is shown below.  It does 
exactly what you just described.  So there is nothing left to be done here on 
the cookie side.  So the only things I can do now are either close this bug out 
as wont-fix or reassign this bug to to the networking group to finish up on.

---------------

    // Get the original url that the user either typed in or clicked on
    nsCOMPtr<nsILoadGroup> pLoadGroup;
    rv = aHttpChannel->GetLoadGroup(getter_AddRefs(pLoadGroup));
    if (NS_FAILED(rv)) return rv;
    nsCOMPtr<nsIChannel> pChannel;
    if (pLoadGroup) {
      nsCOMPtr<nsIRequest> pRequest;
      rv = pLoadGroup->GetDefaultLoadRequest(getter_AddRefs(pRequest));
      if (NS_FAILED(rv)) return rv;
      pChannel = do_QueryInterface(pRequest);
    }
    nsCOMPtr<nsIURI> pFirstURL;
    if (pChannel) {
      rv = pChannel->GetURI(getter_AddRefs(pFirstURL));
    } else {
      rv = aHttpChannel->GetURI(getter_AddRefs(pFirstURL));
    }
    if (NS_FAILED(rv)) return rv;

Comment 29

[Darin Fisher]

morse: gagan meant to say that this bug is blocked on bug 87731, which belongs to
pavlov.  perhaps this should be a dupe of that bug instead??

Comment 30

[Stephen P. Morse]

That sounds like a good solution.  Because once bug 87731 is fixed, there should 
be nothing further to do here.

*** This bug has been marked as a duplicate of 87731 ***

Comment 31

[Stephen P. Morse]

*** Bug 102092 has been marked as a duplicate of this bug. ***

Comment 32

[Daniel Veditz [:dveditz]]

This problem still exists, reopening.

Fixing bug 87731 might magically make this start working again, but we really
can't be sure it's not masking other bugs until it's cleared. QA verifying bug
87731 wouldn't necessarily think to re-check 3rd party cookies. Plus with this
closed you're just going to gather duplicates from folks who search for cookie
bugs before filing but for some reason don't think to read all imagelib bugs.

Comment 33

[Stephen P. Morse]

Marking as 9.6 but that will slip if bug 87731 slips.

Comment 34

[Peter Trudelle]

sounds too risky for the benefit at this point. nsbranch-

Comment 35

[Jeremy M. Dolan]

There's no way to just grab the URI in the location bar? All you need is the
domain from that. It's updated for redirects too. If you can't access that,
whatever C++ code is setting that, have it set something the cookie code can
access too.

When a script sets a cookie, we're going to need to check it too, to see if the
script is local.

Comment 36

[J Luh]

Bug 87388 was marked "won't fix"--does Mozilla intend to drop support for
third-party cookie blocking?

Comment 37

[Stephen P. Morse]

If bug 87388 is not fixed, there is no way for the cookie module to be able to 
determine if a cookie is a third-party cookie or not.

Comment 38

[Stephen P. Morse]

Worse yet, all the p3p code that was just added depends on our being able to 
determine if the cookie is from a third party.  So all that code will not work 
correctly.

cc'ing harishd and barrowma for the p3p implications of this.

Comment 39

[Daniel Veditz [:dveditz]]

You mean bug 87731 was marked WONTFIX, and note that "Mozilla" has no intentions
whatsoever. Some developers value this feature, and certain others--who broke it
in the first place--obviously don't value it if it costs them any work. Never
mind that security and privacy is one of the few compelling benefits of Mozilla
over IE that end-users (e.g. my Mom) can understand.

Comment 40

[Judson Valeski]

calling this critical.

Comment 41

[Stephen P. Morse]

This bug used to be blocked by 87731.  If bug 87731 is not going to be fixed, 
there needs to be some other way for the cookie module to determine third-party 
cookies.  So I just opened bug 107579 for that ability.  Bug 107579 is now 
blocking this bug.

As mentioned above, this bug will slip from 9.6 if bug 87731 slips from 9.6.    
Since that just happened, changing the target milestone of this bug to 9.8, but 
it will slip out further depending on what target milestone is assigned to bug 
107579.

Comment 42

[Stephen P. Morse]

Really setting target milestone to 0.9.8 this time.  (I thought I did it 
yesterday but apparently I didn't.)

Comment 43

[Jeremy M. Dolan]

fixing spello in summary for search friendliness

Comment 44

[Stephen P. Morse]

So you changed "third-party cookie" to "third party cookie" claiming it was a 
spelling error otherwise.  Turns out it was correct the original way because it 
avoids any possible mis-reading.  But if it helps for searching, then I have no 
objections with leaving out the hyphen.

Comment 45

[Jeremy M. Dolan]

> claiming it was a spelling error otherwise

According to "Merriam-Webster OnLine" and Wordnet, there is no hyphen. I changed
a few other bugs so now searching for "third party" in the summary as a
substring should return them all. (the bigger problem were ones with stuff like
"3rd party")

Of course, this *is* from a dictionarary that calls itself "OnLine", which
doesn't instill a great deal of confidence in its definitions. 

Comment 46

[Peter Trudelle]

removing vestigial dependencies.  This is blocked by bug 107579 alone.

Comment 47

[Stephen P. Morse]

Blocker bug 107579 just got deferred to 0.9.9.  So this bug must do the same.

Comment 48

[Stephen P. Morse]

*** Bug 121271 has been marked as a duplicate of this bug. ***

Comment 49

[Christopher Aillon (sabbatical, not receiving bugmail)]

Making the summary more query friendly

Comment 50

[Stephen P. Morse]

So you changed the title from 

   third party cookie blocking doesn't work

to

   third party cookie blocking doesn't work when "Enable cookies for the
   originating Web site only" is set

Well if that makes you happy, then I'll leave it that way.  But it makes me 
wonder as to when else we could possibly have third-party cookie blocking.

Comment 51

[Stephen P. Morse]

Note going to make it in 0.9.9.  Moving out to 1.0

Comment 52

[Stephen P. Morse]

Posting a patch that correctly recognizes third-party cookies.  This patch 
requires that the patch that I posted for bug 107579 be applied as well (it is 
not yet checked in).

To test this out, apply both patches (the one here and the one in bug 107579), 
then run the tests at either of the following and note the printf output on the 
consule

   http://warp/u/morse/cookies/  -- inside netscape firewall
   http://people.netscape.com/morse/cookies/ -- outside firewall

The reason I am giving an address inside the firewall is that there is currently 
an error in the external version and the correction that I posted for it won't 
be visible outside the firewall until tomorrow.

Comment 53

[Stephen P. Morse]

Attachment: properly detects third party cookies in all cases

Comment 54

[Stephen P. Morse]

I forgot to mention that before running the test, you need to change DEBUG_morse 
to just DEBUG in nscookies.cpp.

Comment 55

[Daniel Veditz [:dveditz]]

Comment on attachment 70505 [details] [diff] [review]
properly detects third party cookies in all cases

r=dveditz

Comment 56

[Stephen P. Morse]

Oops,I forgot about the http request.  The previous patch was for the 3rd-party 
test on the http response only.  Attaching a new patch that does the same sort 
of thing for the request as well.

Comment 57

[Stephen P. Morse]

Attachment: same patch extended to handle http request as well as response

Comment 58

[Stephen P. Morse]

*** Bug 67447 has been marked as a duplicate of this bug. ***

Comment 59

[Darin Fisher]

Comment on attachment 70771 [details] [diff] [review]
same patch extended to handle http request as well as response

>Index: nsCookies.cpp

>+  char* firstSpec = nsnull;
>+    rv = pFirstURL->GetSpec(&firstSpec);
>+  PR_FREEIF(firstSpec);

how about using nsXPIDLCString instead?

    nsXPIDLCString firstSpec;

      rv = pFirstURL->GetSpec(getter_Copies(firstSpec));

otherwise, sr=darin

Comment 60

[Stephen P. Morse]

Darin, I originally tried to use an nsXPIDLCString for firstSpec.   Problem is 
that firstSpec is then passed as an argument to COOKIE_SetCookieStringFromHttp 
and that routine has it declared as a char* rather than a const char*.  Yes, it 
really should be changed to const char*, but then there are other ripple effects 
of the change that will permeate throughout the module.  So what started out as 
a simple change to this file will result in a substantial rewrite of the file 
with the risk of introducing regressions.

Comment 61

[Darin Fisher]

oh right... i forgot about that.  i hit the same problem while working on some
upcoming nsIURI changes for bug 124042 :(

we really should fix the const'ness of those methods at some point.

Comment 62

[Peter Trudelle]

nsbeta1+

Comment 63

[Daniel Veditz [:dveditz]]

Comment on attachment 70771 [details] [diff] [review]
same patch extended to handle http request as well as response

r=dveditz

Comment 64

[Asa Dotzler [:asa]]

Comment on attachment 70771 [details] [diff] [review]
same patch extended to handle http request as well as response

a=asa (on behalf of drivers) for checkin to 0.9.9 and the mozilla trunk

Comment 65

[Brendan Eich [:brendan]]

>>+  char* firstSpec = nsnull;
>>+    rv = pFirstURL->GetSpec(&firstSpec);
>>+  PR_FREEIF(firstSpec);
>>
>>how about using nsXPIDLCString instead?
>
>... Problem is that firstSpec is then passed as an argument to 
>COOKIE_SetCookieStringFromHttp and that routine has it declared as a char* 
>rather than a const char*.

So use nsXPIDLCString firstSpec; as Darin suggested, then pass
NS_CONST_CAST(char *, firstSpec.get()) to the routines that should be
constipated, but aren't yet.

Also, in the "not that we care for 1.0, but why make things worse for later,
when we'll have even more places to clean up", don't use PR_FREEIF -- use
nsMemory::Free if you can't use nsXPIDLCString.  But use nsXPIDLCString with
const_cast'ed .get()s, because automatic storage management wins every time, and
cleaning up the const_casts entails surveying exactly the COOKIE_* APIs that
need to be constipated, and not rewriting a bunch of manual-allocation code yet
again.

I think this should be done now, not later -- morse, are you game?  If you run
into some roadblock, give a shout here, I'll read my cc-list bugmail.

/be

Comment 66

[Stephen P. Morse]

Attachment: Same patch but with nsXPIDLCString per Brendan's request

Comment 67

[Brendan Eich [:brendan]]

Comment on attachment 72197 [details] [diff] [review]
Same patch but with nsXPIDLCString per Brendan's request

Cool, thanks -- all review and approval marks should carry forward if it
compiles and runs as before.

/be

Comment 68

[Stephen P. Morse]

Fix checked in.

Comment 69

[Jeremy M. Dolan]

Tested with branch 2002030414, with both cookies and images set to 'only from
originating server'.

Went to cnn.com. toolbar images from toolbar.netscape.com are loaded, as well as
an ad from ar.atwola.com. Headline image from i.cnn.net corrently NOT loaded.

Went to msn.com. No problems.

Went to time.com. Cookie for .pathfinder.com ACCEPTED. Same wrong images as
cnn.com accepted.

(I believe you duped the 3rd paty image bugs here too)

Comment 70

[Stephen P. Morse]

That's not a valid test.  Did you look at the traffic to see how those cookies 
got set?  There are many known ways for a third-party site to look like a 
first-party site so that they can set cookies.  We are not trying to stop that 
(there is no way that we can).  What this bug is trying to do is to duplicate 
the behavior of 4.x so that overt third-party cookies won't get set.

As an example of what I'm referring to, sites have learned that they can get 
around the third-party restriction by doing a redirect to the ad server, passing 
in their own URL in the query string.  The ad server is now a first-party site 
(that's what redirect means), it sets a cookie, and then it redirects back to 
the site whose URL is in the query string it received.

Comment 71

[Jeremy M. Dolan]

wget time.com shows no redirect to pathfinder.com for the main page. It does one
redirect, to another page on time.com. I don't see why a redirect on an images
would be able to get around this.

% wget time.com
HTTP request sent, awaiting response... 302 Moved Temporarily
Location: http://www.time.com/ [following]
HTTP request sent, awaiting response... 302 Moved Temporarily
Location: http://www.time.com/time/ [following]
HTTP request sent, awaiting response... 200 OK
Length: unspecified [text/html]

What about the image issues, there shouldn't be any such backdoor WRT third
party image loading. Is that supposed to be fixed with the new capabilities from
107579 yet?

Comment 72

[Tom Everingham]

verified - win NT4, linux rh6, mac osX