Closed Bug 1891349 (CVE-2024-5699) Opened 3 months ago Closed 3 months ago

Firefox doesn't treat cookie name prefixes as case-insensitive

Categories

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

Firefox 126
defect

Tracking

()

RESOLVED FIXED
127 Branch
Tracking Status
firefox127 + fixed

People

(Reporter: preissa, Assigned: longsonr)

References

(Blocks 1 open bug)

Details

(Keywords: reporter-external, sec-low, Whiteboard: [necko-triaged][adv-main127+])

Attachments

(3 files)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:124.0) Gecko/20100101 Firefox/124.0

Steps to reproduce:

Hi there, I'm sorry if this is not the right location to report this.

"draft-ietf-httpbis-rfc6265bis-13" (Cookies: HTTP State Management Mechanism) [1] says at "5.4. Cookie Name Prefixes":

User agents' requirements for cookie name prefixes differ slightly
from servers' (Section 4.1.3) in that UAs MUST match the prefix
string case-insensitively.

The normative requirements for the prefixes are detailed in the
storage model algorithm defined in Section 5.6.

This is because some servers will process cookie case-insensitively,
resulting in them unintentionally miscapitalizing and accepting
miscapitalized prefixes.

For example, if a server sends the following Set-Cookie header field

Set-Cookie: __SECURE-SID=12345

to a UA which checks prefixes case-sensitively it will accept this
cookie and the server would incorrectly believe the cookie is subject
the same guarantees as one spelled __Secure-.

(This section is present since draft version 11 ("draft-ietf-httpbis-rfc6265bis-11"), published on 2022-11-07.)

One such example of a server/framework is ASP.NET Core, where a code like

context.Request.Cookies.TryGetValue("__Secure-SID", ...)

will also return a cookie whose name is "__SECURE-SID", "__secure-SID" etc.

However, I noticed Firefox Nightly (version 126.0a1 (2024-04-13)) seems to still treat the cookie prefixes "__Host-" and "__Secure-" as case-sensitive. For example, if I send the following header in a HTTP response (with the correct "__Secure-" prefix):

Set-Cookie: __Secure-SID=123456; path=/; samesite=lax; httponly

then Firefox correctly rejects the cookie as it's missing the "secure" attribute, as shown in the warning logged to the console:

Cookie “__Secure-SID” has been rejected for invalid prefix.

However, if I send the following header in the HTTP response:

Set-Cookie: __secure-SID=123456; path=/; samesite=lax; httponly

then Firefox doesn't log a warning and instead accepts the cookie. However, this means servers that treat cookie names case-insensitively, as shown above, might think the cookie name matches "__Secure-SID" and might therefore rely on the UA to only have accepted it if it has the "secure" attribute and was sent over a secure origin, which isn't the case here.

In contrast, Chromium seems to check the prefix with a case-insensitive comparison for the prefix name [2] (depending on the net::features::kCaseInsensitiveCookiePrefix feature, which seems to be enabled by default [3]).

Is there a plan to also implement a case-insensitive check for the cookie name prefixes soon? Otherwise, the security guarantees intended by the cookie name prefixes currently doesn't seem to be provided by Firefox for web servers/frameworks that treat cookie names as case-insensitive.

(As a precaution, I checked the "security issue" checkbox, as maybe this could be treated as a security issue.)

Thanks!

[1] https://datatracker.ietf.org/doc/draft-ietf-httpbis-rfc6265bis/13/
[2] https://source.chromium.org/chromium/chromium/src/+/main:net/cookies/canonical_cookie.cc;l=409-416?q=canonical_cookie.cc&ss=chromium%2Fchromium%2Fsrc
[3] https://source.chromium.org/chromium/chromium/src/+/main:net/base/features.cc;l=303-305?q=kCaseInsensitiveCookiePrefix&ss=chromium%2Fchromium%2Fsrc

Actual results:

When sending the following header in a HTTP response:

Set-Cookie: __secure-SID=123456; path=/; samesite=lax; httponly

Firefox accepted the cookie, even though it misses the "secure" attribute and "draft-ietf-httpbis-rfc6265bis-13" requires the cookie prefix name ("__Secure-") to be matched case-insensitively

Expected results:

When sending the following header in a HTTP response:

Set-Cookie: __secure-SID=123456; path=/; samesite=lax; httponly

Firefox should have rejected the cookie, because it misses the "secure" attribute and "draft-ietf-httpbis-rfc6265bis-13" requires the cookie prefix name ("__Secure-") to be matched case-insensitively.

Sorry, I forgot to include a reference about ASP.NET Core: This code [1] (RequestCookieCollection) creates the store (Dictionary<string, string>) for cookie names and values, specifying StringComparer.OrdinalIgnoreCase for matching the keys, which means cookie names are matched case-insensitively.

[1] https://github.com/dotnet/aspnetcore/blob/81355450754f0e6471244b7f1f457b866e982a15/src/Http/Http/src/Internal/RequestCookieCollection.cs#L28-L36

For reference, the change in Chromium that introduced a case-insensitive match of cookie name prefixes is: https://issues.chromium.org/issues/40236421

Relevant issue in whatwg/http-extensions: https://github.com/httpwg/http-extensions/issues/2231

Thanks!

I thought we had fixed this but apparently not:
https://searchfox.org/mozilla-central/rev/b1cffa04131a77e5e118d86d6366e0d660a3072b/netwerk/cookie/CookieService.cpp#2061-2062

We should use strncasecmp there for a quick fix.
I notice we have two copies of the prefix string constants in this file so it would be nice to clean up the duplication between CookieService::CheckHiddenPrefix() and CookieService::CheckPrefixes(), both called sequentially from CookieService::CanSetCookie()

There's probably a dupe on this because surely we're failing the WPT tests that landed for this spec change. In any case the issue is essentially public

Group: firefox-core-security
Component: Untriaged → Networking: Cookies
Keywords: sec-low
Product: Firefox → Core
Blocks: cookie
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: nobody → longsonr
Status: NEW → ASSIGNED
Severity: -- → S2
Priority: -- → P2
Whiteboard: [necko-triaged][necko-priority-new]
Whiteboard: [necko-triaged][necko-priority-new] → [necko-triaged]
Pushed by longsonr@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/715ecd9aaf75
Treat cookie name prefixes as case-insensitive r=dveditz,cookie-reviewers,valentin
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 127 Branch
Blocks: 1891941

Thank you for quickly fixing this!
(According to the tests dashboard, Safari now seems to be the only browser which doesn't yet treat cookie name prefixes case-insensitively.)

Pushed by imoraru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/48ea6b3c1cc7
set cookies/prefix/__host.header.https.html as passing test steps on Windows x86

A patch has been attached on this bug, which was already closed. Filing a separate bug will ensure better tracking. If this was not by mistake and further action is needed, please alert the appropriate party. (Or: if the patch doesn't change behavior -- e.g. landing a test case, or fixing a typo -- then feel free to disregard this message)

Regressions: 1892450
Whiteboard: [necko-triaged] → [necko-triaged][adv-main127+]
Alias: CVE-2024-5699
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: