1756213 - document.cookie skips "schemeful" samesite checks in nested documents

Status: NEW

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

Description

[Daniel Veditz [:dveditz]]

Spun off from bug 1748693 comment 12

Both CookieService::GetCookieStringFromDocument and CookieServiceChild::GetCookieStringFromDocument (used for document.cookie) are not correctly accounting for the "schemeful" checks. Bug 1748693 is removing the MaybeCompareSchemes calls that were based on a stored "schemeMap" value (not spec compliant) and blocked too many cookies. But it also didn't block some cookies it should in the case of nested documents. The following existing check is basically what we want, and in the right place, but needs to account for "schemeful"

    if (thirdParty &&
        !CookieCommons::ShouldIncludeCrossSiteCookieForDocument(cookie)) {
      continue;
    }

The pref-dependent scheme check needs to be built into the concept of thirdParty here, but unfortunately that's calculated with our standard ThirdPartyUtils that only compare domains schemelessly. We can't change the basic thirdPartyUtils call to consider schemes because it's used for cookies blocking, tracking protection, and other things that need to remain schemeless for now. Could

• add an optional dontIgnoreScheme argument or parallel methods to the thirdpartyutils, but that could get pretty deep.
• Maybe just walk the window tree in a new special-purpose CookieCommon function so the changes stay local. if ( (thirdParty || isNestedCrossScheme() ) &&

The latter means walking the tree a second time (after the thirdParty check) which might not be that nice. I have no idea whether the perf cost would be noticeable or whether people use document.cookie in nested documents enough for it to be noticed.

Obviously the first thing in that proposed function would be to bail out and return false if the schemeful pref is turned off

Comment 1

[Tom Schuster (MoCo)]

I think this might actually be the cause of one of the test failures that shows up with D136229.

TestCookie.TestCookieMain | Value of: CheckResult(cookie.get(), MUST_CONTAIN, "test=security3")

First the cookie is set for some https URL and later we get load the http version of that page a lookup document.cookie.

Comment 2

[Tom Schuster (MoCo)]

I finally managed to reproduce this, thanks to timhuang. The way we partition third-party storage actually makes reproducing this bug difficult. The third-party iframe would have to call requestStorageAccess, otherwise the cookie list it gets is empty anyway.

Updating ThirdPartyUtil::IsThirdPartyWindow to add support for "dontIgnoreScheme" seems difficult, because we don't actually compute the third-partyness in that function at all. Instead we just lookup the precomputed value from WindowContext::GetIsThirdPartyWindow.

Comment 3

[Tom Schuster (MoCo)]

Attachment: WIP: Bug 1756213 - Check scheme for cookie with SameSite