Opened 19 months ago

Closed 17 months ago

Last modified 11 months ago

#26158 closed Bug (fixed)

cookie parsing fails with python 3.x if request contains unnamed cookie

Reported by: Andreas Dolk Owned by: Tim Graham
Component: HTTP handling Version: 1.8
Severity: Normal Keywords: cookie python3
Cc: will@…, zachborboa@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Example:

  from http import cookies
  good_cookie = 'csrftoken=5lkzK7FCI2iWy2xi7wbZPI7P26qbspIE; django_language=en'
  print(cookies.parse_cookie(good_cookie))
  bad_cookie = 'csrftoken=5lkzK7FCI2iWy2xi7wbZPI7P26qbspIE; unnamed; django_language=en'
  print(cookies.parse_cookie(bad_cookie))

Output:

  {'csrftoken': '5lkzK7FCI2iWy2xi7wbZPI7P26qbspIE', 'django_language': 'en'}
  {}

This parsing method is used during processing WSGI requests (see: django.core.handlers.wsgi.py)

If a request contains at least one nameless cookie, then the parser result is an empty dictionary. The nameless cookie may be present in the client (browser) and this cause at least two serious problems for django:

  • django will not see the session cookie (user is asked to login again)
  • csrf validation fails with an error message (csrf cookie not set - because the middleware doesn't see it)

It worked, btw., with python 2.7.

Remark:
Nameless cookies shouldn't happen - but in our case there's a different web application on the parent domain that sometimes sets a nameless cookie. Even though that this is a bug, it makes our django app quite unusable due to the cookie parsing and we'd have to ask the users to constantly delete the cookies on their browsers (or avoid the other app). Those cookies are evil but we have to expect them in real life :/

Change History (13)

comment:1 Changed 19 months ago by Tim Graham

This is likely due to https://hg.python.org/cpython/rev/9e765e65e5cb. A related report in Django is #25458, but I transferred it to a Python ticket: https://bugs.python.org/issue25228. I guess any changes to mitigate this issue should probably handled in Python as well. I'm not sure it can be fixed without reintroduced the security issue fixed in Python, but my understanding of the issue is limited.

comment:2 Changed 19 months ago by Andreas Dolk

Django relies on the python core module http.cookies for parsing the cookie strings. Right now, django apps that run with python 3.x are vulnerable and stop working, if the request contains a nameless cookie. It would be great, if the python devs accepted this as a bug (i.e. ignore unnamed cookies and create all the others instead on none). But there's no guarantee and in the end, it's not python but our django app that stops working and my clients complain.

An alternative could be to split the cookie at ';' and load the cookies one by one. As far as I understand, the correct cookies will be created, the unnamed ones ignored.

So instead of doing

  c.load(cookie)

something like

  [c.load(_cookie) for _cookie in cookie.split(';')]

(Just for the idea, don't know if escaped semicolons are allowed in cookie values - but it would do the trick of filtering the unnamed cookies and keeping the correct ones)

Last edited 19 months ago by Andreas Dolk (previous) (diff)

comment:3 Changed 19 months ago by Sergei Maertens

As mentioned in the upstream ticket, blindly splitting on ; could be a problem because an attacker might sent semicolons inside the cookie itself. Example from upstream: name="some;value".

comment:4 Changed 19 months ago by Tim Graham

Owner: changed from nobody to Tim Graham
Status: newassigned
Triage Stage: UnreviewedAccepted

I'll assign this to myself for further investigation.

comment:5 Changed 19 months ago by Will Harris

Cc: will@… added

comment:6 Changed 19 months ago by Zach Borboa

Cc: zachborboa@… added

comment:7 Changed 18 months ago by Collin Anderson

It should be safe to hard split on semicolon. name="some;value" is not valid, even though it's quoted. I think raw double quotes, commas, semicolons and backslashes are _always_ invalid characters in cookie values.

From https://tools.ietf.org/html/rfc6265:

 cookie-value      = *cookie-octet / ( DQUOTE *cookie-octet DQUOTE )
 cookie-octet      = %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E
                       ; US-ASCII characters excluding CTLs,
                       ; whitespace DQUOTE, comma, semicolon,
                       ; and backslash

comment:8 Changed 17 months ago by Tim Graham

Has patch: set
Patch needs improvement: set

comment:9 Changed 17 months ago by Collin Anderson

Patch needs improvement: unset

comment:10 Changed 17 months ago by Tim Graham

Triage Stage: AcceptedReady for checkin

comment:11 Changed 17 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 93a135d:

Fixed #26158 -- Rewrote http.parse_cookie() to better match browsers.

comment:12 Changed 11 months ago by Tim Graham <timograham@…>

In d1bc980d:

[1.9.x] Fixed CVE-2016-7401 -- Fixed CSRF protection bypass on a site with Google Analytics.

This is a security fix.

Backport of "refs #26158 -- rewrote http.parse_cookie() to better match
browsers." 93a135d111c2569d88d65a3f4ad9e6d9ad291452 from master

comment:13 Changed 11 months ago by Tim Graham <timograham@…>

In 6118ab7d:

[1.8.x] Fixed CVE-2016-7401 -- Fixed CSRF protection bypass on a site with Google Analytics.

This is a security fix.

Backport of "refs #26158 -- rewrote http.parse_cookie() to better match
browsers." 93a135d111c2569d88d65a3f4ad9e6d9ad291452 from master

Note: See TracTickets for help on using tickets.
Back to Top