1473011 - Cookies code should pass mDocumentURI instead of mURI to ThirdPartyUtil third party checks

Status: RESOLVED INVALID

(Core :: Networking: Cookies, enhancement, P3)

Description

[:Gijs (he/him)]

(Filed based on discussion in bug 1469916; ping me if you need me to CC you.)

It seems there's some confusion about the URIs/channels the cookie services uses for third party checks.

https://searchfox.org/mozilla-central/rev/14cb8f1c238735ba1abe18ad44e39808983c2572/netwerk/cookie/nsICookieService.idl#126-127,143

seems to suggest that the URI passed to getCookieStringFromHttp should be the document's URI. As far as I can tell there's only 1 non-test callsite,

https://searchfox.org/mozilla-central/rev/97d488a17a848ce3bebbfc83dc916cf20b88451c/netwerk/protocol/http/HttpBaseChannel.cpp#3438

which passes `mURI` for the channel - which is the URI of the request/channel in question, not the document URI which triggered the channel/request.

This is potentially problematic because if a document on foo.com loads a script for bar.com, and third-party cookies were blocked, I'd expect us to not send cookies on that request because bar.com is third-party to foo.com .

This confusion also potentially impacts samesite cookie restrictions in some cases.

I don't know either the cookie, the third-party cookie, the http channel, or the samesite cookie implementation very well, so it's quite possible I'm missing something here, but the discussion in bug 1469916 seemed to suggest other people agree this is potentially problematic.

The other thing I noticed here is that we do a number of checks for third-party-ness, some of which seem redundant if we passed the right information at the beginning (ie we use the loading principal of the channel to find the document URI and then do *another* check to see if *that*'s third party - whereas if we'd passed that URI to begin with then presumably that check wouldn't be necessary?).

Comment 1

[Andrea Marchesini [:baku]]

The API has changed drastically in the last 8 years. The 3rd-party checks are executed by ThirdPartyUtil::AnalyzeChannel which uses the loadInfo from the channel to analyze the context.