Opened 8 years ago

Closed 8 years ago

Last modified 8 years 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 by Tim Graham, 8 years ago

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 by Andreas Dolk, 8 years ago

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 ends, it's not python but our django app (and our clients) that stops working...

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)

Version 0, edited 8 years ago by Andreas Dolk (next)

comment:3 by Sergei Maertens, 8 years ago

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 by Tim Graham, 8 years ago

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

I'll assign this to myself for further investigation.

comment:5 by Will Harris, 8 years ago

Cc: will@… added

comment:6 by Zach Borboa, 8 years ago

Cc: zachborboa@… added

comment:7 by Collin Anderson, 8 years ago

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 by Tim Graham, 8 years ago

Has patch: set
Patch needs improvement: set

comment:9 by Collin Anderson, 8 years ago

Patch needs improvement: unset

comment:10 by Tim Graham, 8 years ago

Triage Stage: AcceptedReady for checkin

comment:11 by Tim Graham <timograham@…>, 8 years ago

Resolution: fixed
Status: assignedclosed

In 93a135d:

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

comment:12 by Tim Graham <timograham@…>, 8 years ago

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 by Tim Graham <timograham@…>, 8 years ago

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