Exception when http.parse_cookie recieves bad cookie

[Fredrik Stålnacke]

Hi!

I'm currently having a problem where the user in some circumstances get a strange cookie (from a sub-domain that I don't control). When this cookie is parsed by django http.parse_cookie an exception occurs that causes the user to get a error message back.

I've managed to reproduce the error with the following code:

from django import cookie
evil_cookie="""test=<#?xml version="1.0" encoding="utf-16"?#><#CookieData xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema"#><#/CookieData#>|<#?xml version="1.0" encoding="utf-16"?#><#CookieData xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema"#><#/CookieData#>"""
http.parse_cookie(evil_cookie)

The error message that are created are:

python2.6/site-packages/django/http/__init__.pyc in parse_cookie(cookie)
    461         try:
    462             c = SimpleCookie()
--> 463             c.load(cookie, ignore_parse_errors=True)
    464         except Cookie.CookieError:
    465             # Invalid cookie


python2.6/site-packages/django/http/__init__.pyc in load(self, rawdata, ignore_parse_errors)
     95                     self.bad_cookies = []
     96                     self._BaseCookie__set = self._loose_set
---> 97                 super(SimpleCookie, self).load(rawdata)
     98                 if ignore_parse_errors:
     99                     self._BaseCookie__set = self._strict_set

/System/Library/Frameworks/Python.framework/Versions/2.6/lib/python2.6/Cookie.pyc in load(self, rawdata)
    623         """
    624         if type(rawdata) == type(""):
--> 625             self.__ParseString(rawdata)
    626         else:
    627             self.update(rawdata)

/System/Library/Frameworks/Python.framework/Versions/2.6/lib/python2.6/Cookie.pyc in __ParseString(self, str, patt)
    654             else:
    655                 rval, cval = self.value_decode(V)
--> 656                 self.__set(K, rval, cval)
    657                 M = self[K]
    658     # end __ParseString


python2.6/site-packages/django/http/__init__.pyc in _loose_set(self, key, real_value, coded_value)
    105             def _loose_set(self, key, real_value, coded_value):
    106                 try:
--> 107                     self._strict_set(key, real_value, coded_value)
    108                 except Cookie.CookieError:
    109                     self.bad_cookies.append(key)

/System/Library/Frameworks/Python.framework/Versions/2.6/lib/python2.6/Cookie.pyc in __set(self, key, real_value, coded_value)
    576         """Private method for setting a cookie's value"""
    577         M = self.get(key, Morsel())
--> 578         M.set(key, real_value, coded_value)
    579         dict.__setitem__(self, key, M)
    580     # end __set


AttributeError: 'NoneType' object has no attribute 'set'

I am using Django 1.3 on Mac OS X with Python 2.6.1

[Frank Hoffsümmer]

patched exception handling in parse_cookie method

[Aymeric Augustin]

I am not sure about what happens exactly. However, I confirmed that the bug occurs with Django's patched version of SimpleCookie and not with Python's original version.

>>> from Cookie import SimpleCookie
>>> SimpleCookie().load(evil_cookie)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/opt/local/Library/Frameworks/Python.framework/Versions/2.6/lib/python2.6/Cookie.py", line 627, in load
    self.__ParseString(rawdata)
  File "/opt/local/Library/Frameworks/Python.framework/Versions/2.6/lib/python2.6/Cookie.py", line 660, in __ParseString
    self.__set(K, rval, cval)
  File "/opt/local/Library/Frameworks/Python.framework/Versions/2.6/lib/python2.6/Cookie.py", line 580, in __set
    M.set(key, real_value, coded_value)
  File "/opt/local/Library/Frameworks/Python.framework/Versions/2.6/lib/python2.6/Cookie.py", line 455, in set
    raise CookieError("Illegal key value: %s" % key)
Cookie.CookieError: Illegal key value: xmlns:xsi

(that exception would be catched properly in Django).

The patch does not seem appropriate. We should fix the root cause and not mask it by arbitrarily catching an exception.

[vung]

Fix failure when a bad cookie name is repeated

[vung]

This is related to #13007.

Here is a short example:

from django import http
http.parse_cookie('a:=b; a:=c; d=e')

The problem is that when a CookieError is raised http.SimpleCookie._loose_set bypasses regular code paths to store a key whose value is None. The normal code path would ensure that the value is a Morsel object.

None works fine when the key occurs only once, so this isn't catched by the test commited in r15523.

When the same key is encountered a second time, though, this value is used in BaseCookie under the assumption that it is a Morsel instance and consequently it has a set() method. Of course, None doesn't have one, hence the bug.

The immediate fix is to use a Morsel instance. It doesn't matter if it supports httponly or not, it will be removed anyway.

Fixing this brings a second problem: bad cookies are colected in a list, to be removed when loading finishes. This will result in calling del self[key] more than once for the same key and will fail.

15852_repeated_bad_key.diff is a short patch that fixes the above.

[Ramiro Morales]

In [16452]:

(The changeset message doesn't reference this ticket)

[joonas.kuorilehto@…]

I believe this bug has not been fixed in 1.3.X branch. As I have understood bug fixes like this should land into 1.3.X.

I was testing with Codenomicon HTTP Test Suite fuzzer and noticed a very similar traceback in my server logs. I can reproduce this by sending the following minimized HTTP request:

POST / HTTP/1.1
Host: 10.10.3.83
Cookie: = = = = =

{}

This causes a 'NoneType' 'AttributeError' on both Django test server and Apache mod_wsgi.

Traceback with Django 1.3.1:

Traceback (most recent call last):
  File "/var/env/local/lib/python2.7/site-packages/django/core/servers/basehttp.py", line 283, in run
    self.result = application(self.environ, self.start_response)
  File "/var/env/local/lib/python2.7/site-packages/django/contrib/staticfiles/handlers.py", line 68, in __call__
    return self.application(environ, start_response)
  File "/var/env/local/lib/python2.7/site-packages/django/core/handlers/wsgi.py", line 272, in __call__
    response = self.get_response(request)
  File "/var/env/local/lib/python2.7/site-packages/django/core/handlers/base.py", line 169, in get_response
    response = self.handle_uncaught_exception(request, resolver, sys.exc_info())
  File "/var/env/local/lib/python2.7/site-packages/django/core/handlers/base.py", line 218, in handle_uncaught_exception
    return callback(request, **param_dict)
  File "/var/env/local/lib/python2.7/site-packages/django/utils/decorators.py", line 89, in _wrapped_view
    result = middleware.process_view(request, view_func, args, kwargs)
  File "/var/env/local/lib/python2.7/site-packages/django/middleware/csrf.py", line 116, in process_view
    request.META["CSRF_COOKIE"] = _sanitize_token(request.COOKIES[settings.CSRF_COOKIE_NAME])
  File "/var/env/local/lib/python2.7/site-packages/django/core/handlers/wsgi.py", line 218, in _get_cookies
    self._cookies = http.parse_cookie(self.environ.get('HTTP_COOKIE', ''))
  File "/var/env/local/lib/python2.7/site-packages/django/http/__init__.py", line 468, in parse_cookie
    c.load(cookie, ignore_parse_errors=True)
  File "/var/env/local/lib/python2.7/site-packages/django/http/__init__.py", line 97, in load
    super(SimpleCookie, self).load(rawdata)
  File "/usr/lib/python2.7/Cookie.py", line 632, in load
    self.__ParseString(rawdata)
  File "/usr/lib/python2.7/Cookie.py", line 665, in __ParseString
    self.__set(K, rval, cval)
  File "/var/env/local/lib/python2.7/site-packages/django/http/__init__.py", line 107, in _loose_set
    self._strict_set(key, real_value, coded_value)
  File "/usr/lib/python2.7/Cookie.py", line 585, in __set
    M.set(key, real_value, coded_value)
AttributeError: 'NoneType' object has no attribute 'set'

I think this problem is related to this ticket. Either the fix is not included in the 1.3.X branch or the fix is incomplete.

[Aymeric Augustin]

Until a few months ago, all bugfixes were backported to the stable branch. This policy was recently changed in order to reduce the risk of regressions and the work load for the core team.

The new policy is documented here: ​https://docs.djangoproject.com/en/dev/internals/release-process/#supported-versions

[Steven Cummings]

Replying to aaugustin:

Until a few months ago, all bugfixes were backported to the stable branch. This policy was recently changed in order to reduce the risk of regressions and the work load for the core team.

The new policy is documented here: ​https://docs.djangoproject.com/en/dev/internals/release-process/#supported-versions

So that policy is clear. My follow-up question would be: what makes this "normal", or otherwise not "critical" in severity (which would get it into the 1.3.X branch by your policy).

My perspective on this bug, is that it would be nice if there were no reproducible ways for any kind of website user (web browser, api consumer, etc.) to cause a 500 error. OTOH, I don't have an idea of how many more tickets that would put into the same category to backport.

So, what is the criteria for various severity levels? If that's documented somewhere, I couldn't find it.

[Luke Plant]

The criteria are given on the page already linked:

• Security issues.
• Data-loss bugs.
• Crashing bugs.
• Major functionality bugs in newly-introduced features.

The rule of thumb is that fixes will be backported to the last minor release for bugs that would have prevented a release in the first place.

Having thought about this, it seems that this would fall under 'crashing bugs' and under the last point. This causes crashes, and since this is a regression relative to Python's SimpleCookie, I think I would have marked this as a release blocker had it been found before 1.3 was released.

[Aymeric Augustin]

Given Luke's analysis, and since the patch applies easily to the 1.3 branch, I've backported the fix in r17168.