httpwg / http-extensions

HTTP Extensions in progress
https://httpwg.org/http-extensions/
445 stars 146 forks source link

[rfc6265bis] nameless cookies, client/server inconsistencies #2229

Open lavish opened 2 years ago

lavish commented 2 years ago

I am reporting on some inconsistencies discovered while researching browser/server compliance with rfc6265bis.

rfc6265bis-04 changed the cookie parsing algorithm to support nameless cookies, i.e., Set-Cookie: token should create a nameless cookie with value token. The commit that introduced the change is https://github.com/httpwg/http-extensions/commit/0178223fb44fb194930f53181c5852954890e233, which followed the discussion at https://github.com/httpwg/http-extensions/issues/159.

The relevant parts are in Section 5.4: The Set-Cookie Header Field, point 3.

If the name-value-pair string lacks a %x3D ("=") character, then the name string is empty, and the value string is the value of name-value-pair. Otherwise, the name string consists of the characters up to, but not including, the first %x3D ("=") character, and the (possibly empty) value string consists of the characters after the first %x3D ("=") character.

And in Section 5.6.3: Retrieval Algorithm, point 4.

  1. If the cookies' name is not empty, output the cookie's name followed by the %x3D ("=") character.
  2. If the cookies' value is not empty, output the cookie's value.
  3. If there is an unprocessed cookie in the cookie-list, output the characters %x3B and %x20 ("; ").

Conflict with the BNF syntax

These changes seem to be incompatible with the Set-Cookie BNF syntax in Section 4.1.1, where the = symbol is considered mandatory in the cookie-pair definition: cookie-pair = cookie-name BWS "=" BWS cookie-value.

Inconsistent browser behavior

According to my tests, recent versions of Chrome and Firefox follow the latest rfc6265bis, while Safari deviates from the intended behavior. Setting a cookie via document.cookie = "test" on example.com results into the following:

Browser Cookie Name Cookie Value document.cookie getter HTTP request header
Chrome test 'test' Cookie: test
Firefox test 'test' Cookie: test
Safari test 'test=' Cookie: test=

Client/Server confusion: cookie tossing

Major server-side programming languages and web development frameworks are not compatible with nameless cookies as specified in rfc6265bis. For instance, PHP and Flask-based applications would treat the request header Cookie: test as a cookie named test with an empty value.

This discrepancy between browsers and servers has real-world security implications. Consider a site at https://example.com that relies on the integrity of __Host- cookies to, e.g., implement CSRF protections:

  1. Assume that https://example.com sets the cookie __Host-xsrf=foo
  2. An attacker controlling http://sub.example.com (either directly or via an XSS) sets a cookie in the victim's browser via document.cookie = '__Host-xsrf; Domain=example.com; Path=/folder'
  3. Subsequent requests to https://example.com/folder will contain the HTTP request header Cookie: __Host-xsrf; __Host-xsrf=foo
  4. The PHP backend will populate the $_COOKIE associative array as Array([__Host-xsrf] => ), effectively ignoring the legitimate cookie in favor of the malicious one.

The attack flow described above defeats double-submit CSRF protections relying on __Host- cookies for additional security against same-site attackers.

Notice that:

  1. Safari is not affected by this attack since the restrictions of cookie name prefixes apply
  2. PHP seems to reject nameless cookies in the form Cookie: =foo

Proposed solution

Difficult to say since there are discrepancies at multiple levels (specs, browsers, servers). If nameless cookies must be supported for backward compatibility, the specification could mandate the presence of the = symbol in the name-value-pair produced by the retrieval algorithm. This change should hopefully make the Cookie header easier to parse, clarifying potential ambiguities at the server-side.

sbingler commented 2 years ago

Hi

Thank you for your (well researched) issue. As you've found out, nameless cookies are quite the footgun.

These changes seem to be incompatible with the Set-Cookie BNF syntax in Section 4.1.1

Section 4 describes what a well behaved server should do. Unfortunately not every server is well behaved which is why Section 5 describes a parsing algorithm that accepts it. https://github.com/httpwg/http-extensions/pull/1018

Inconsistent browser behavior

I'm not sure if this is intentional, https://github.com/httpwg/http-extensions/issues/159#issuecomment-571378104 is ambiguous and I don't have access to the radar issue. @johnwilander What was the intended implementation?

Client/Server confusion: cookie tossing

This is a big problem when it comes to cookie prefixes. I can say that Chrome is already aware and has a fix in the works.

Proposed solution

Yep, this isn't an easy problem to solve. Nameless cookies are a footgun and servers are strongly advised to never use them or accept them.

There was a previous discussion around appending all nameless cookies with = but such a sweeping change is likely to cause breakages and was rejected.

lavish commented 2 years ago

Hi and thanks for the reply!

I realized another (more severe) security implication. To avoid disclosing the issue publicly, I filed 2 identical reports to Firefox (#1783982) and Chrome (#1351601). Safari does not seem to be affected.