1892748 - Reject most control characters in cookie attributes

Status: RESOLVED FIXED

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

Description

[Robert Longson [:longsonr]]

We ignore null characters in cookies but https://www.rfc-editor.org/rfc/rfc2616.html#section-3.3.1 does not treat null any differently from any other control character.
Cookie data that contains control characters should be ignored.

Comment 1

[Robert Longson [:longsonr]]

Attachment: Bug 1892748 Part 1 - Reject control characters in cookie attributes. r=dveditz

Comment 2

[Robert Longson [:longsonr]]

Attachment: Bug 1892748 Part 2 - Reject cookies that end in a terminator character. r=dveditz

Comment 3

[Robert Longson [:longsonr]]

https://treeherder.mozilla.org/jobs?repo=try&revision=677e2650ca4fe102b6d7217f3931e9c83d747fab

Comment 4

[Daniel Veditz [:dveditz]]

(In reply to Robert Longson [:longsonr] from comment #0)

We ignore null characters in cookies but https://www.rfc-editor.org/rfc/rfc2616.html#section-3.3.1 does not treat null any differently from any other control character.
Cookie data that contains control characters should be ignored.

Nobody ever implemented that 1999 spec as written; it's the wrong specification to reference. It was obsoleted by the 2011 rfc6265, created to document reality as much as possible and with cross-vendor agreements and compromises where implementations differed. Another dozen years later and we're actually aiming our implementation at the in-progress update to 6265, supported by a whole raft of Web Platform Tests to achieve and maintain cross-vendor interoperability.

That said, this bug report is correct: we should rejecting cookies with control characters other than HTAB anywhere in the cookie string according to step 1 at https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis-13#section-5.5

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3d8324880910
Part 1 - Reject control characters in cookie attributes. r=dveditz,cookie-reviewers
https://hg.mozilla.org/integration/autoland/rev/11097fcc4f76
Part 2 - Reject cookies that end in a terminator character. r=dveditz,cookie-reviewers

Comment 6

[Tom Schuster (MoCo)]

Shouldn't we have a pref for this?

Comment 7

[Robert Longson [:longsonr]]

Safari and Chrome already pass these WPTs so it seems unlikely we'd have any incompatibilities.

Comment 8

[tszentpeteri]

https://hg.mozilla.org/mozilla-central/rev/3d8324880910
https://hg.mozilla.org/mozilla-central/rev/11097fcc4f76

Comment 9

[Daniel Veditz [:dveditz]]

(In reply to Robert Longson [:longsonr] from comment #7)

Safari and Chrome already pass these WPTs so it seems unlikely we'd have any incompatibilities.

😔

We said the same thing about "lax by default" and "none requires secure", too, but that didn't stop some web sites from sending different cookies to Firefox that broke things just the same.