Cookie totally breaks if the client sets a bare cookie

[xmcp]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

A cookie may be not in the key=value format. For example. if the JS code runs document.cookie = "bad", it becomes:

I don't know how to call it. I will use the term "bare cookie" in this report. In the following requests with a bare cookie, the Cookie HTTP header becomes: Cookie: key=value; bad

It seems that Sanic cannot parse the header with bare cookies, and will throw all cookies (including the legimit key=value pair) away. See the code snippet below.

Code snippet

from sanic import Sanic
from sanic.response import html, text

app = Sanic("test")
app.config.AUTO_EXTEND = False

@app.get("/")
async def route1(request):
    return html('<script>document.cookie="key=value"; document.cookie="bad"; location.href="/fire";</script>')
    
@app.get("/fire")
async def route2(request):
    return text(f'''
        headers = {request.headers.get("Cookie")}
        key = {request.cookies.get("key", "none")}
    ''')

if __name__ == '__main__':
    app.run(port=4321, debug=True)

Then visit http://127.0.0.1:4321/ in Chrome. The page shows:

headers = key=value; bad
key = none

Expected Behavior

The page should show:

headers = key=value; bad
key = value

How do you run Sanic?

As a script (app.run or Sanic.serve)

Operating System

Windows

Sanic Version

22.12.0

Additional context

I am using the latest stable Chrome (117.0.5938.150) to reproduce this.

[Tronic]

Are you aware if this cookie format is allowed by any specs, or if it simply a quirk that the browser can send it like that? Browsers otherwise do some formatting on document.cookie and don't simply pass the string.

Malformed cookie headers could be used for exploiting parsers which understand them in different ways, potentially altering content of another cookie, so we try to make Sanic err on the side of rejection all of it rather than potentially return manipulated values, although it is noted that setting a cookie to prevent other cookies being parsed can also be exploited.

Should we simply skip the "bare cookie" and still try to read the rest, or store it as a "" named cookie value? Needs investigation. Any real world implications to this?

[xmcp]

See httpwg/http-extensions#159 and httpwg/http-extensions#1018. It seems that document.cookie = "foo" is considered as a Cookie of a null key and a foo value.

The impact to my website is that if a frontend script accidentially sets things like document.cookie = "x", the user will never be able to log in, because Sanic will refuse to read any other values in the cookie.

[Tronic]

I believe Sanic since 23.3 should simply ignore that one cookie and not the rest (because we re-did cookie parsing for that release), so you should be fine simply upgrading to the latest. However, since the specs call for allowing it, I've made a PR to have 23.9 or 23.12 onwards also reading the value of cookies that are nameless (empty string, not None, to avoid breakage).

The new patch supports both =value and value formats, which apparently do exist in the wild.

[Tronic]

Thanks for a high quality bug report! If you are interested on development itself, we would love your PRs on Sanic. There are a lot of minor things like this to look at. Let us know if the 23.x releases already solve the issue.