1951750 - CookieStore set: cookies set from subdomains are host-scoped to (only) eTLD+1, even with the __Host- prefix; document.cookie & cookieStore.getAll() differ

Status: RESOLVED FIXED

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

Description

[Uku Sõrmus]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:136.0) Gecko/20100101 Firefox/136.0

Steps to reproduce:

1. Go to https://y.x.response.ee/
2. Open devtools (F12), Console tab
3. Execute the following JavaScript:

await cookieStore.set("__Host-x", "y")

4. Go to https://response.ee/
5. Open devtools (F12), Storage tab

Actual results:

See that the cookie is automatically set for whole site-scope (response.ee) from y.x.response.ee, even with __Host- prefix, not only for the host that set them (y.x.response.ee)

Expected results:

The default should be no Domain attribute, like with document.cookie or Set-Cookie.
Try the same from y.x.response with:

• document.cookie = "a=b" - only scoped to y.x.response.ee (see Storage tab)
• document.cookie = "__Host-z=w; Secure; Path=/" - only scoped to y.x.response.ee (Domain attribute can't even be used)

Having __Host- prefix with Domain attribute breaks the security assumptions of the __Host- prefix, see also https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#attributes

__Host- prefix: Cookies with names starting with __Host- are sent only to the host subdomain or domain that set them, and not to any other host. They [...] must not have a domain specified [...]

Comment 1

[Uku Sõrmus]

Latest Firefox 136.0 on Ubuntu 22.04, 64-bit
"Mozilla/5.0 (X11; Linux x86_64; rv:136.0) Gecko/20100101 Firefox/136.0"

Comment 2

[Uku Sõrmus]

Elaborating a bit, the new default behavior is not even like setting the Domain attribute, but instead like setting the cookie on eTLD+1 without the Domain attribute (but there's a plot-twist).
The new cookie, set from a subdomain, is only accessible with document.cookie on the eTLD+1, not the subdomain itself (should never happen)... but, cookieStore.getAll() tells a different story!
Cookie header contents sent out on HTTP requests seems to agree with document.cookie here.

On https://www.example.com/ (subdomain)

// Setting cookies...
document.cookie = "a=b"
await cookieStore.set("x", "y")  // should be equal to last statement
document.cookie = "c=d; Domain=example.com"

console.log(document.cookie)  // same as what is sent out with requests in `Cookie` header
// a=b; c=d
console.log(await cookieStore.getAll())  // should be equal to last statement, but...
/*
[
  {
    "name": "a",
    "value": "b"
  },
  {
    "name": "x",
    "value": "y"
  },
  {
    "name": "c",
    "value": "d"
  }
]
*/

Then on https://example.com/ (eTLD+1)

console.log(document.cookie)  // same as what is sent out with requests in `Cookie` header
// x=y; c=d
console.log(await cookieStore.getAll())    // should be equal to last statement, but:
/*
[
  {
    "name": "a",
    "value": "b"
  },
  {
    "name": "x",
    "value": "y"
  },
  {
    "name": "c",
    "value": "d"
  }
]
*/

One way of impact is that subdomain arbitrary cookie write (most likely via XSS) can help an attacker override __Host- prefixed cookies on eTLD+1 host, forfeiting the host-scoped security guarantee of __Host- prefixed cookies. E.g., from 1.dev.example.com -> example.com

// 1. On https://example.com
document.cookie = "__Host-test=a; Path=/; Secure" 
// 2. On https://www.example.com
await cookieStore.set("__Host-test", "b")
// 3. On https://example.com again
console.log(document.cookie)
// "__Host-test=b"
// ^ overridden!

Comment 3

[Daniel Veditz [:dveditz]]

CookieStore.cpp is making up it's own validation for cookie things that don't match what we do for normal HTTP cookies, For example, basic name validity checking:
https://searchfox.org/mozilla-central/rev/cb46268bc26b0cd9e91e625aa92aaa5a6f047b9d/dom/cookiestore/CookieStore.cpp#45-53
vs.
https://searchfox.org/mozilla-central/rev/cb46268bc26b0cd9e91e625aa92aaa5a6f047b9d/netwerk/cookie/CookieCommons.cpp#219-262

Ed: could you take a look here and see what else is missing?

Comment 4

[Daniel Veditz [:dveditz]]

We need to test for ALL of the rules to make sure they're covered. Every cookie correctness test needs to have a corresponding CookieStore equivalent. Is the secure flag required for prefix cookies? Can you set a bogus path for __Host- cookies? overwrite an HttpOnly cookie? Overwrite a secure cookie from a non-secure site? etc.

Comment 5

[Valentin Gosu [:valentin] {{ PTO 3 July - 3 August }}]

(In reply to Daniel Veditz [:dveditz] from comment #4)

We need to test for ALL of the rules to make sure they're covered. Every cookie correctness test needs to have a corresponding CookieStore equivalent. Is the secure flag required for prefix cookies? Can you set a bogus path for __Host- cookies? overwrite an HttpOnly cookie? Overwrite a secure cookie from a non-secure site? etc.

I've had a look with Ed at the main differences in validation. It does seem we missed a few for the cookie store and would be good to centralize checks.

• Nameless cookies that have a value starting with a prefix check here that's causing bug 1951746.
• The fact that the domain of the cookie should be the domain of the site when it starts with __Host check here that is directly causing this bug.
• Path containing \t characters check here
• parameters like path and domain containing control characters

I think we shouldn't have any issues with secure cookies, since cookie store is only available on secure origins.

Comment 6

[Valentin Gosu [:valentin] {{ PTO 3 July - 3 August }}]

Also, I agree that we should centralize the cookie validity checks after plugging these holes.
Instead of using AddNative it should go through a common method that checks the cookieStruct, similar to SetCookieStringFromHttp and AddCookieFromDocument

Comment 7

[Andrea Marchesini [:baku]]

I don't know how much we can centralize because CookieStore API has its own set of rules. They should be compatible, but not exactly like the cookie specs.

Comment 8

[edgul]

Maybe a good compromise would be a shared "validator" component/function that contains all of the shared checks. And make the non-shared checks as explicit as possible to avoid future misplaced shared checks.

Comment 9

[edgul]

I think CTL filtering needs some centralization for sure. Putting this here for now, but probably will handle in follow up.

CookieStore:

• We only check names and values for CTLs

CookieParser:

• CheckName has strictest CTL filtering
• CheckValue has secondmost strict CTL filtering
• CheckCookieStringFromDocument seemd to check the whole cookie string, but only on document.cookie
• ContainsControlCharacters is seems to check the whole cookie string.
• Parser used by document.cookie and HTTP cookie setting

Comment 10

[edgul]

So in addition to the checking on non-name/value attributes, I think cookie store is missing 0x3D filtering on name.

Comment 11

[Valentin Gosu [:valentin] {{ PTO 3 July - 3 August }}]

Path containing \t characters check here
parameters like path and domain containing control characters
(In reply to Daniel Veditz [:dveditz] from bug 373228 comment #1)
#1 introduction
cookies are stored in a tab-delimited file with 7 fields. We're interested
in the PATH of the cookie.

#2 Injection
If you put a tab character in the path then you break the file format and
can hack the interpretation of fields after that point. (variations)

I think this is no longer the case and we can remove the \t path restriction that regular cookies have? 🤔

Also, in light of https://wicg.github.io/cookie-store/#issue-d93c44be I'm not sure whether we should apply the character checks to these attributes.

Note that it’s up for discussion whether these character restrictions should also apply to expires, domain, path, and sameSite as well.

Comment 12

[Valentin Gosu [:valentin] {{ PTO 3 July - 3 August }}]

Daniel, any thoughts on comment 11?

Comment 13

[Valentin Gosu [:valentin] {{ PTO 3 July - 3 August }}]

Attachment: Bug 1951750 - Handle domain argument in CookieStoreParent r=baku,edgul

Comment 14

[Daniel Veditz [:dveditz]]

(In reply to Andrea Marchesini [:baku] from comment #7)

I don't know how much we can centralize because CookieStore API has its own set of rules. They should be compatible, but not exactly like the cookie specs.

If the rules for CookieStore don't match the rules enforced for setting document.cookie that will open sites to attacks. If the CookieStore spec allows it then the spec is broken and needs to be fixed.

Comment 15

[Daniel Veditz [:dveditz]]

Daniel, any thoughts on comment 11?

We no longer have our own internal reasons for forbidding tabs. They are still forbidden by the conservative section 4 of the cookie spec
https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis-19#:~:text=av-octet%20%20%20%20%20%20%20%20%20%20=
but the more liberal rules in section 5 require user agents to support them
https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis-19#name-the-set-cookie-header-field

I don't feel strongly about it, but compatibility with other UAs is likely best. Do WPT tests check for this? I do feel strongly that whatever we think the rules for cookies are, CookieStore should not violate it. The API actually restricts certain things you could do with document.cookie, like there's no way to create a cookie without a Secure attribute. That's fine, as long as its a strict subset and not an expansion.

Also, in light of https://wicg.github.io/cookie-store/#issue-d93c44be I'm not sure whether we should apply the character checks to these attributes.

That issue/note is not even consistent with its own algorithm: step 19 says

Perform the steps defined in Cookies § Storage Model for when the user agent "receives a cookie" ...

which would by definition apply all those checks. In practice I bet everyone takes short-cuts and doesn't actually concatenate a cookie string and then re-parse the string (like we call nsICookieManager::AddNative), but it should get the same results which means yes, applying those character checks. And honestly, if it's not a performance bottleneck it really might be safest to do literally what the spec says and call CookieCommons::CreateCookieFromDocument(). Syntax checks aren't the only restrictions that need to be duplicated otherwise.

Comment 16

[Uku Sõrmus]

Since I can't see Phabricator revisions, it might be fixed already, but here's another precedent where unified checks between Set-Cookie / document.domain and CookieStore could be beneficial. If shall be opened as separate bug, please ping.

Whitespace is allowed where it previously wasn't. Here, I have only tried SP (U+0020).

PoC on https://example.com:

/* --- Old API --- */
list = ["", "  ", "=", " =", "= ", " = "]
for (i of list) {
  document.cookie = i
}

document.cookie
// ""
// ^ none set


/* --- New API --- */
await cookieStore.set("", "").catch(e => console.log(e))
// TypeError: Cookie name and value both cannot be empty
await cookieStore.set("", " ").catch(e => console.log(e))
await cookieStore.set(" ", "").catch(e => console.log(e))
await cookieStore.set("   ", "   ").catch(e => console.log(e)) 

await cookieStore.getAll()
/*
[
  {
    "name": "",
    "value": " "
  },
  {
    "name": " ",
    "value": " "
  },
  {
    "name": "   ",
    "value": "   "
  }
]
*/
document.cookie
// " ;  =;    =   "

Security impact not immediately clear, but I image this could confuse some server-side parsers? All of these fail on Chrome (Version 136.0.7054.0 (Official Build) canary (64-bit)).

Quoting RFC6265bis (emphasis mine):

5.6. The Set-Cookie Header Field
...
In addition, the algorithm below also strips leading and trailing whitespace from the cookie name and value (but maintains internal whitespace), whereas the grammar in Section 4.1 forbids whitespace in these positions [at all]

Comment 17

[Valentin Gosu [:valentin] {{ PTO 3 July - 3 August }}]

Attachment: Bug 1951750 - Add cookie store WPT r=baku

Comment 18

[Uku Sõrmus]

In the bug [meta] Cookie Store API, I can see a reference to a public bug 1953589 cookieStore and document.cookie should share same source of truth.

There seems to be some confusion there about reproducibility, but I think it's the same behavior I have already outlined here in comment 2 and boils down to security-impactful wrong behavior around handling same-site context / subdomains in Cookie Store API. The wrong behavior doesn't expose itself when trying to reproduce it only on a single bare domain (eTLD+1) like example.com, without the www..

---

At least two problems, to reiterate and elaborate:

1. If you set a cookie with cookieStore.set from any level of subdomain (e.g. www.example.com), it is hostname-scoped to eTLD+1 (e.g. example.com) instead, from the perspective of the old API (document.cookie read & Cookie request header sent out)

   • that is a security problem since it happens even with the __Host- prefix; (sub)domains should never be able to overwrite cookies with the __Host- prefix from any other hostname than the hostname (exact domain or IP) that set it. And without the __Host- prefix on the original cookie, subdomains should only be able to add another "competing" cookie with the same name (utilizing the Domain attribute), but still not overwrite the original cookie
   • PoC:
     1. go to https://www.example.com/
     2. execute in console await cookieStore.set("a", "b")
     3. execute in console console.log(document.cookie) and observe there is no cookie visible (the same emptiness as in the Storage tab of devtools)
     4. go to https://example.com/
     5. execute in console console.log(document.cookie) and observe that the cookie is visible, and as such, host-scoped to parent domain instead (also see the Storage tab of devtools which shows example.com in the Domain column)

2. A cookie set (with old OR the new API) on any subdomain in the same-site context, with OR without the Domain attribute, are visible to all domains in the same-site context, using cookieStore.getAll

   • which is an even bigger security problem, since cookies without Domain attribute (including with __Host- name prefix, that's one of the premises of that prefix) should be host-scoped and not readable from any other hostname (subdomain or parent domain in the same-site context)
   • PoC:
     1. go to https://x.response.ee/
     2. execute in console document.cookie = "__Host-SESSIONID=secret; Secure; Path=/" and observe the cookie is correctly set
     3. open tabs to both https://y.x.response.ee/ and https://response.ee/
     4. in both, execute in console console.log(document.cookie) and observe that it is empty
     5. in both, execute in console await cookieStore.getAll() and observe that we can read the cookie that is hostname-scoped to another domain

---

"Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:136.0) Gecko/20100101 Firefox/136.0" with about:config dom.cookieStore.enabled = true

Comment 19

[Uku Sõrmus]

To add oil to the fire, using the domain attribute in the setter doesn't work with the CookieStore API either, it only seems to support the eTLD+1 without leading period and even that hostname-scopes the cookie (as seen by the old API), not making it available to the entire same-site scope like expected behavior.

On https://y.x.response.ee/:

await cookieStore.set({name: "a", value: "b", domain: "y.x.response.ee"}).catch(e => console.error(e)) 
// TypeError: Cookie domain must domain-match current host
await cookieStore.set({name: "a", value: "b", domain: "x.response.ee"}).catch(e => console.error(e))
// TypeError: Cookie domain must domain-match current host
await cookieStore.set({name: "a", value: "b", domain: ".response.ee"}).catch(e => console.error(e))
// TypeError: Cookie domain cannot start with '.'
await cookieStore.set({name: "a", value: "b", domain: "response.ee"}).catch(e => console.error(e))
// succesful, but not set correctly; only available on response.ee, not its subdomains

v.s.

document.cookie = "foo=a; Domain=y.x.response.ee";  // set correctly
document.cookie = "bar=b; Domain=.x.response.ee";  // set correctly; note that .domain.tld and domain.tld are equivalent
document.cookie = "baz=c; domain=response.ee";  // set correctly

console.log(document.cookie)
// "foo=a; bar=b; baz=c"

Also, there's no error when using the domain attribute with TLD from public suffix list (PSL), although it doesn't seem to actually set the Cookie. On https://mozilla.github.io/:

document.cookie = "a=b; Domain=github.io"
// Cookie “a” has been rejected for invalid domain.

await cookieStore.set({name: "a", value: "b", domain: "github.io"}).catch(e => console.error(e))
// (no error thrown)

These two and the whitespace bug could be slashed to separate issues, but it starts to feel more and more like a the current Cookie Store API implementation is waiting a bigger rewrite anyways?

Comment 21

[Valentin Gosu [:valentin] {{ PTO 3 July - 3 August }}]

The cookieStore always using the baseDomain is the reason why we disabled the cookie store through a pref flip on Thursday and through a Firefox dot release next week.
I have some WIP patches that will address all of these problems, though a follow-up to centralize the cookie setting will probably be necessary.

Comment 22

[Frederik Braun [:freddy]]

@Uku Sõrmus: Thank you for this bug report. We will fix the bugs in their own time in Firefox Nightly and piece by piece.

In the meantime (to add to Valentin's comment), we have fully disabled the cookie store API in Firefox release and beta versions by default.
Our ability to disable APIs remotely depends on a setting that a few users may have opted out of in advanced privacy settings (under "studies"). Those will have to wait for 136.0.2 which will release later this week.

This was all done under bug 1953368, which will become public in due time.

We will issue a CVE and credit you for this security bug, once the bugs have been fully remediated and shipped to end users (currently targeting Firefox 138 but TBD). This will take some time.

Comment 23

[Uku Sõrmus]

Sure, thanks for letting me know

Comment 24

[Valentin Gosu [:valentin] {{ PTO 3 July - 3 August }}]

Attachment: Bug 1951750 - Expose CheckCookieStruct function so we can run cookie validity checks on CookieStruct r=edgul,baku

Comment 25

[Valentin Gosu [:valentin] {{ PTO 3 July - 3 August }}]

Attachment: Bug 1951750 - Don't allow space in cookieStore names or values r=baku

Comment 26

[Valentin Gosu [:valentin] {{ PTO 3 July - 3 August }}]

Attachment: Bug 1951750 - WPT for cookieStore with spaces in name or value r=baku

Comment 27

[edgul]

Can we also check once CookieStoreManager impl lands that the baseDomain check in CookieStoreSubscriptionService::Observe behaves as expected?

See this comment for location.

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f3959de161ba
Expose CheckCookieStruct function so we can run cookie validity checks on CookieStruct r=edgul,baku,cookie-reviewers
https://hg.mozilla.org/integration/autoland/rev/123069093c2f
Handle domain argument in CookieStoreParent r=baku,edgul,cookie-reviewers
https://hg.mozilla.org/integration/autoland/rev/e0cfc652fcc6
Don't allow space in cookieStore names or values r=baku

Comment 29

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for causing build bustages @TestCookie.cpp and lint failure.

• Backout link
• Push with failures
• Failure Log lint failures
• Failure Log build bustage

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ee233c9ac108
Expose CheckCookieStruct function so we can run cookie validity checks on CookieStruct r=edgul,baku,cookie-reviewers
https://hg.mozilla.org/integration/autoland/rev/bb99a3d0d1e7
Handle domain argument in CookieStoreParent r=baku,edgul,cookie-reviewers
https://hg.mozilla.org/integration/autoland/rev/4374d7fb7f40
Don't allow space in cookieStore names or values r=baku

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ee233c9ac108
https://hg.mozilla.org/mozilla-central/rev/bb99a3d0d1e7
https://hg.mozilla.org/mozilla-central/rev/4374d7fb7f40

Comment 32

[Uku Sõrmus]

Note that "Don't allow space in cookieStore names or values" conflicts with the permissiveness of document.cookie setter, i.e. the behaviors don't align:

document.cookie = "a b=c d"  // set successfully
await cookieStore.set("e f", "g").catch(e => console.error(e))  // TypeError: Cookie name contains invalid chars
await cookieStore.set("h", "i j").catch(e => console.error(e))  // TypeError: Cookie value contains invalid chars

---

Firefox Nightly 140.0a1 (2025-05-02) (64-bit)

Comment 33

[Phabricator Automation]

Comment on attachment 9472679 [details]
Bug 1951750 - WPT for cookieStore with spaces in name or value r=baku

Revision D241968 was moved to bug 1965382. Setting attachment 9472679 [details] to obsolete.

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7e092800aacd
Add cookie store WPT r=baku

Comment 35

[Donal Meehan [:dmeehan]]

https://bugzilla.mozilla.org/show_bug.cgi?id=1951750

Comment 36

[Uku Sõrmus]

(In reply to Frederik Braun [:freddy] from comment #22)

@Uku Sõrmus: Thank you for this bug report. We will fix the bugs in their own time in Firefox Nightly and piece by piece.

In the meantime (to add to Valentin's comment), we have fully disabled the cookie store API in Firefox release and beta versions by default.
[...]

We will issue a CVE and credit you for this security bug, once the bugs have been fully remediated and shipped to end users (currently targeting Firefox 138 but TBD). This will take some time.

Hi there, for historic purposes and research credit, why wasn't this and a few other security bugs assigned CVEs?

The list of five cookie-related security bugs I reported ~ one year ago that have been fixed:
https://bugzilla.mozilla.org/buglist.cgi?bug_id=1951746%2C1951750%2C1964216%2C1964767%2C1964945&list_id=17869492

Of the five, "only" one of the bugs was assigned CVE-2025-8037: 1964767 - Nameless cookies can be used to shadow Secure cookies from an insecure connection. (Another bug from the list above, 1964945 - Nameless cookie can impersonate HttpOnly cookie that has the same domain, host-only-flag, path was considered a duplicate of 1964767, but the CVE description had no mention of that bug.)

I would consider that this bag of issues here (1951750) was actually much more severe than the other issues reported, from preconditions and impact perspective, but got no mention. Especially the behavior that cookieStore.getAll had (the second point outlined in comment 18). So that left me wondering what's the decision process around CVE assignment.

Cheers

Comment 37

[Frederik Braun [:freddy]]

This was fixed in Firefox before the feature was available to the general population in a released version. The feature shipped in Firefox 138 with your patch applied and was disabled before that. We only assign CVEs for issues that affect the real product.