[Django] #26158: cookie parsing fails with python 3.x if request contains unnamed cookie
Django
-------------------------------+----------------------------
Reporter: Treverix | Owner: nobody
Type: Bug | Status: new
Component: HTTP handling | Version: 1.8
Severity: Normal | Keywords: cookie python3
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+----------------------------
Example:
{{{#!python
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 :/
--
Ticket URL: <https://code.djangoproject.com/ticket/26158>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Treverix | Owner: nobody
Type: Bug | Status: new
Component: HTTP handling | Version: 1.8
Keywords: cookie python3 | Triage Stage: Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26158#comment:1>
Django
Reporter: Treverix | Owner: nobody
Type: Bug | Status: new
Component: HTTP handling | Version: 1.8
Keywords: cookie python3 | Triage Stage: Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (by Treverix):
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
{{{#!python
c.load(cookie)
}}}
something like
{{{#!python
[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)
--
Ticket URL: <https://code.djangoproject.com/ticket/26158#comment:2>
Django
Reporter: Treverix | Owner: nobody
Type: Bug | Status: new
Component: HTTP handling | Version: 1.8
Keywords: cookie python3 | Triage Stage: Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (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"`.
--
Ticket URL: <https://code.djangoproject.com/ticket/26158#comment:3>
Django
Reporter: Treverix | Owner: timgraham
Type: Bug | Status: assigned
Component: HTTP handling | Version: 1.8
Keywords: cookie python3 | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: new => assigned
* owner: nobody => timgraham
* stage: Unreviewed => Accepted
Comment:
I'll assign this to myself for further investigation.
--
Ticket URL: <https://code.djangoproject.com/ticket/26158#comment:4>
Django
Reporter: Treverix | Owner: timgraham
Type: Bug | Status: assigned
Keywords: cookie python3 | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by willharris):
* cc: will@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/26158#comment:5>
Django
Reporter: Treverix | Owner: timgraham
Type: Bug | Status: assigned
Keywords: cookie python3 | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by zachborboa):
* cc: zachborboa@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/26158#comment:6>
Django
Reporter: Treverix | Owner: timgraham
Type: Bug | Status: assigned
Keywords: cookie python3 | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by collinanderson):
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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26158#comment:7>
Django
Reporter: Treverix | Owner: timgraham
Type: Bug | Status: assigned
Keywords: cookie python3 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
Comment:
Collin proposed to [https://github.com/django/django/pull/6277 remove
Django's reliance on Python's cookie parsing].
--
Ticket URL: <https://code.djangoproject.com/ticket/26158#comment:8>
Django
Reporter: Treverix | Owner: timgraham
Type: Bug | Status: assigned
Keywords: cookie python3 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by collinanderson):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/26158#comment:9>
Django
Reporter: Treverix | Owner: timgraham
Type: Bug | Status: assigned
Keywords: cookie python3 | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/26158#comment:10>
Django
Reporter: Treverix | Owner: timgraham
Component: HTTP handling | Version: 1.8
Keywords: cookie python3 | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"93a135d111c2569d88d65a3f4ad9e6d9ad291452" 93a135d]:
{{{
#!CommitTicketReference repository=""
revision="93a135d111c2569d88d65a3f4ad9e6d9ad291452"
Fixed #26158 -- Rewrote http.parse_cookie() to better match browsers.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26158#comment:11>
Django
Reporter: Andreas Dolk | Owner: Tim
| Graham
Type: Bug | Status: closed
Component: HTTP handling | Version: 1.8
Keywords: cookie python3 | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"d1bc980db1c0fffd6d60677e62f70beadb9fe64a" d1bc980d]:
{{{
#!CommitTicketReference repository=""
revision="d1bc980db1c0fffd6d60677e62f70beadb9fe64a"
[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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26158#comment:12>
Django
Reporter: Andreas Dolk | Owner: Tim
| Graham
Type: Bug | Status: closed
Keywords: cookie python3 | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"6118ab7d0676f0d622278e5be215f14fb5410b6a" 6118ab7d]:
{{{
#!CommitTicketReference repository=""
revision="6118ab7d0676f0d622278e5be215f14fb5410b6a"
[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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26158#comment:13>