From 6118ab7d0676f0d622278e5be215f14fb5410b6a Mon Sep 17 00:00:00 2001 From: Collin Anderson Date: Fri, 11 Mar 2016 21:36:08 -0500 Subject: [PATCH] [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 --- django/http/cookie.py | 29 +++++++++++---------- docs/releases/1.8.15.txt | 18 +++++++++++++ docs/releases/index.txt | 1 + tests/httpwrappers/tests.py | 52 ++++++++++++++++++++++++++++++++++++- tests/requests/tests.py | 5 +--- 5 files changed, 87 insertions(+), 18 deletions(-) create mode 100644 docs/releases/1.8.15.txt diff --git a/django/http/cookie.py b/django/http/cookie.py index a3dbd2a0b2..decb6db8a7 100644 --- a/django/http/cookie.py +++ b/django/http/cookie.py @@ -89,18 +89,21 @@ else: def parse_cookie(cookie): - if cookie == '': - return {} - if not isinstance(cookie, http_cookies.BaseCookie): - try: - c = SimpleCookie() - c.load(cookie) - except http_cookies.CookieError: - # Invalid cookie - return {} - else: - c = cookie + """ + Return a dictionary parsed from a `Cookie:` header string. + """ cookiedict = {} - for key in c.keys(): - cookiedict[key] = c.get(key).value + if six.PY2: + cookie = force_str(cookie) + for chunk in cookie.split(str(';')): + if str('=') in chunk: + key, val = chunk.split(str('='), 1) + else: + # Assume an empty name per + # https://bugzilla.mozilla.org/show_bug.cgi?id=169091 + key, val = str(''), chunk + key, val = key.strip(), val.strip() + if key or val: + # unquote using Python's algorithm. + cookiedict[key] = http_cookies._unquote(val) return cookiedict diff --git a/docs/releases/1.8.15.txt b/docs/releases/1.8.15.txt new file mode 100644 index 0000000000..e977cffbab --- /dev/null +++ b/docs/releases/1.8.15.txt @@ -0,0 +1,18 @@ +=========================== +Django 1.8.15 release notes +=========================== + +*September 26, 2016* + +Django 1.8.15 fixes a security issue in 1.8.14. + +CSRF protection bypass on a site with Google Analytics +====================================================== + +An interaction between Google Analytics and Django's cookie parsing could allow +an attacker to set arbitrary cookies leading to a bypass of CSRF protection. + +The parser for ``request.COOKIES`` is simplified to better match the behavior +of browsers and to mitigate this attack. ``request.COOKIES`` may now contain +cookies that are invalid according to :rfc:`6265` but are possible to set via +``document.cookie``. diff --git a/docs/releases/index.txt b/docs/releases/index.txt index 070d815ec1..b80ce5830d 100644 --- a/docs/releases/index.txt +++ b/docs/releases/index.txt @@ -25,6 +25,7 @@ versions of the documentation contain the release notes for any later releases. .. toctree:: :maxdepth: 1 + 1.8.15 1.8.14 1.8.13 1.8.12 diff --git a/tests/httpwrappers/tests.py b/tests/httpwrappers/tests.py index dbe85bf3a8..753a7b21d5 100644 --- a/tests/httpwrappers/tests.py +++ b/tests/httpwrappers/tests.py @@ -21,7 +21,7 @@ from django.http import ( from django.test import TestCase from django.utils import six from django.utils._os import upath -from django.utils.encoding import force_text, smart_str +from django.utils.encoding import force_str, force_text, smart_str from django.utils.functional import lazy lazystr = lazy(force_text, six.text_type) @@ -643,6 +643,8 @@ class CookieTests(unittest.TestCase): c2 = SimpleCookie() c2.load(c.output()[12:]) self.assertEqual(c['test'].value, c2['test'].value) + c3 = parse_cookie(c.output()[12:]) + self.assertEqual(c['test'].value, c3['test']) def test_decode_2(self): """ @@ -653,6 +655,8 @@ class CookieTests(unittest.TestCase): c2 = SimpleCookie() c2.load(c.output()[12:]) self.assertEqual(c['test'].value, c2['test'].value) + c3 = parse_cookie(c.output()[12:]) + self.assertEqual(c['test'].value, c3['test']) def test_nonstandard_keys(self): """ @@ -666,6 +670,52 @@ class CookieTests(unittest.TestCase): """ self.assertIn('good_cookie', parse_cookie('a:=b; a:=c; good_cookie=yes').keys()) + def test_python_cookies(self): + """ + Test cases copied from Python's Lib/test/test_http_cookies.py + """ + self.assertEqual(parse_cookie('chips=ahoy; vienna=finger'), {'chips': 'ahoy', 'vienna': 'finger'}) + # Here parse_cookie() differs from Python's cookie parsing in that it + # treats all semicolons as delimiters, even within quotes. + self.assertEqual( + parse_cookie('keebler="E=mc2; L=\\"Loves\\"; fudge=\\012;"'), + {'keebler': '"E=mc2', 'L': '\\"Loves\\"', 'fudge': '\\012', '': '"'} + ) + # Illegal cookies that have an '=' char in an unquoted value. + self.assertEqual(parse_cookie('keebler=E=mc2'), {'keebler': 'E=mc2'}) + # Cookies with ':' character in their name. + self.assertEqual(parse_cookie('key:term=value:term'), {'key:term': 'value:term'}) + # Cookies with '[' and ']'. + self.assertEqual(parse_cookie('a=b; c=[; d=r; f=h'), {'a': 'b', 'c': '[', 'd': 'r', 'f': 'h'}) + + def test_cookie_edgecases(self): + # Cookies that RFC6265 allows. + self.assertEqual(parse_cookie('a=b; Domain=example.com'), {'a': 'b', 'Domain': 'example.com'}) + # parse_cookie() has historically kept only the last cookie with the + # same name. + self.assertEqual(parse_cookie('a=b; h=i; a=c'), {'a': 'c', 'h': 'i'}) + + def test_invalid_cookies(self): + """ + Cookie strings that go against RFC6265 but browsers will send if set + via document.cookie. + """ + # Chunks without an equals sign appear as unnamed values per + # https://bugzilla.mozilla.org/show_bug.cgi?id=169091 + self.assertIn('django_language', parse_cookie('abc=def; unnamed; django_language=en').keys()) + # Even a double quote may be an unamed value. + self.assertEqual(parse_cookie('a=b; "; c=d'), {'a': 'b', '': '"', 'c': 'd'}) + # Spaces in names and values, and an equals sign in values. + self.assertEqual(parse_cookie('a b c=d e = f; gh=i'), {'a b c': 'd e = f', 'gh': 'i'}) + # More characters the spec forbids. + self.assertEqual(parse_cookie('a b,c<>@:/[]?{}=d " =e,f g'), {'a b,c<>@:/[]?{}': 'd " =e,f g'}) + # Unicode characters. The spec only allows ASCII. + self.assertEqual(parse_cookie('saint=André Bessette'), {'saint': force_str('André Bessette')}) + # Browsers don't send extra whitespace or semicolons in Cookie headers, + # but parse_cookie() should parse whitespace the same way + # document.cookie parses whitespace. + self.assertEqual(parse_cookie(' = b ; ; = ; c = ; '), {'': 'b', 'c': ''}) + def test_httponly_after_load(self): """ Test that we can use httponly attribute on cookies that we load diff --git a/tests/requests/tests.py b/tests/requests/tests.py index 75118bb665..f4a8aead92 100644 --- a/tests/requests/tests.py +++ b/tests/requests/tests.py @@ -10,7 +10,7 @@ from django.core.exceptions import SuspiciousOperation from django.core.handlers.wsgi import LimitedStream, WSGIRequest from django.http import ( HttpRequest, HttpResponse, RawPostDataException, UnreadablePostError, - build_request_repr, parse_cookie, + build_request_repr, ) from django.test import RequestFactory, SimpleTestCase, override_settings from django.test.client import FakePayload @@ -161,9 +161,6 @@ class RequestsTests(SimpleTestCase): request = WSGIRequest({'PATH_INFO': wsgi_str("/سلام/"), 'REQUEST_METHOD': 'get', 'wsgi.input': BytesIO(b'')}) self.assertEqual(request.path, "/سلام/") - def test_parse_cookie(self): - self.assertEqual(parse_cookie('invalid@key=true'), {}) - def test_httprequest_location(self): request = HttpRequest() self.assertEqual(request.build_absolute_uri(location="https://www.example.com/asdf"),