innov
/
django1
1
0
Fork 0
Code Issues Pull Requests 1 Projects Releases Wiki Activity

[3.1.x] Fixed CVE-2020-13254 -- Enforced cache key validation in memcached backends.

This commit is contained in:
Dan Palmer 2020-05-20 11:45:31 +02:00 committed by Carlton Gibson
parent 49d7cc19e3
commit 580bd64c04
6 changed files with 66 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
django/core/cache/__init__.py vendored
View File

@ -17,13 +17,13 @@ from asgiref.local import Local
from django.conf import settings
from django.core import signals
from django.core.cache.backends.base import (
BaseCache, CacheKeyWarning, InvalidCacheBackendError,
BaseCache, CacheKeyWarning, InvalidCacheBackendError, InvalidCacheKey,
)
from django.utils.module_loading import import_string
__all__ = [
'cache', 'caches', 'DEFAULT_CACHE_ALIAS', 'InvalidCacheBackendError',
'CacheKeyWarning', 'BaseCache',
'CacheKeyWarning', 'BaseCache', 'InvalidCacheKey',
]
DEFAULT_CACHE_ALIAS = 'default'

33
django/core/cache/backends/base.py vendored
View File

@ -14,6 +14,10 @@ class CacheKeyWarning(RuntimeWarning):
pass
class InvalidCacheKey(ValueError):
pass
# Stub class to ensure not passing in a `timeout` argument results in
# the default timeout
DEFAULT_TIMEOUT = object()
@ -242,18 +246,8 @@ class BaseCache:
backend. This encourages (but does not force) writing backend-portable
cache code.
"""
if len(key) > MEMCACHE_MAX_KEY_LENGTH:
warnings.warn(
'Cache key will cause errors if used with memcached: %r '
'(longer than %s)' % (key, MEMCACHE_MAX_KEY_LENGTH), CacheKeyWarning
)
for char in key:
if ord(char) < 33 or ord(char) == 127:
warnings.warn(
'Cache key contains characters that will cause errors if '
'used with memcached: %r' % key, CacheKeyWarning
)
break
for warning in memcache_key_warnings(key):
warnings.warn(warning, CacheKeyWarning)
def incr_version(self, key, delta=1, version=None):
"""
@ -281,3 +275,18 @@ class BaseCache:
def close(self, **kwargs):
"""Close the cache connection"""
pass
def memcache_key_warnings(key):
if len(key) > MEMCACHE_MAX_KEY_LENGTH:
yield (
'Cache key will cause errors if used with memcached: %r '
'(longer than %s)' % (key, MEMCACHE_MAX_KEY_LENGTH)
)
for char in key:
if ord(char) < 33 or ord(char) == 127:
yield (
'Cache key contains characters that will cause errors if '
'used with memcached: %r' % key, CacheKeyWarning
)
break

17
django/core/cache/backends/memcached.py vendored
View File

@ -4,7 +4,9 @@ import pickle
import re
import time
from django.core.cache.backends.base import DEFAULT_TIMEOUT, BaseCache
from django.core.cache.backends.base import (
DEFAULT_TIMEOUT, BaseCache, InvalidCacheKey, memcache_key_warnings,
)
from django.utils.functional import cached_property
@ -64,24 +66,30 @@ class BaseMemcachedCache(BaseCache):
def add(self, key, value, timeout=DEFAULT_TIMEOUT, version=None):
key = self.make_key(key, version=version)
self.validate_key(key)
return self._cache.add(key, value, self.get_backend_timeout(timeout))
def get(self, key, default=None, version=None):
key = self.make_key(key, version=version)
self.validate_key(key)
return self._cache.get(key, default)
def set(self, key, value, timeout=DEFAULT_TIMEOUT, version=None):
key = self.make_key(key, version=version)
self.validate_key(key)
if not self._cache.set(key, value, self.get_backend_timeout(timeout)):
# make sure the key doesn't keep its old value in case of failure to set (memcached's 1MB limit)
self._cache.delete(key)
def delete(self, key, version=None):
key = self.make_key(key, version=version)
self.validate_key(key)
return bool(self._cache.delete(key))
def get_many(self, keys, version=None):
key_map = {self.make_key(key, version=version): key for key in keys}
for key in key_map:
self.validate_key(key)
ret = self._cache.get_multi(key_map.keys())
return {key_map[k]: v for k, v in ret.items()}
@ -91,6 +99,7 @@ class BaseMemcachedCache(BaseCache):
def incr(self, key, delta=1, version=None):
key = self.make_key(key, version=version)
self.validate_key(key)
# memcached doesn't support a negative delta
if delta < 0:
return self._cache.decr(key, -delta)
@ -109,6 +118,7 @@ class BaseMemcachedCache(BaseCache):
def decr(self, key, delta=1, version=None):
key = self.make_key(key, version=version)
self.validate_key(key)
# memcached doesn't support a negative delta
if delta < 0:
return self._cache.incr(key, -delta)
@ -130,6 +140,7 @@ class BaseMemcachedCache(BaseCache):
original_keys = {}
for key, value in data.items():
safe_key = self.make_key(key, version=version)
self.validate_key(safe_key)
safe_data[safe_key] = value
original_keys[safe_key] = key
failed_keys = self._cache.set_multi(safe_data, self.get_backend_timeout(timeout))
@ -141,6 +152,10 @@ class BaseMemcachedCache(BaseCache):
def clear(self):
self._cache.flush_all()
def validate_key(self, key):
for warning in memcache_key_warnings(key):
raise InvalidCacheKey(warning)
class MemcachedCache(BaseMemcachedCache):
"An implementation of a cache binding using python-memcached"

8
docs/releases/2.2.13.txt
View File

@ -6,6 +6,14 @@ Django 2.2.13 release notes
Django 2.2.13 fixes two security issues and a regression in 2.2.12.
CVE-2020-13254: Potential data leakage via malformed memcached keys
===================================================================
In cases where a memcached backend does not perform key validation, passing
malformed cache keys could result in a key collision, and potential data
leakage. In order to avoid this vulnerability, key validation is added to the
memcached cache backends.
CVE-2020-13596: Possible XSS via admin ``ForeignKeyRawIdWidget``
================================================================

8
docs/releases/3.0.7.txt
View File

@ -6,6 +6,14 @@ Django 3.0.7 release notes
Django 3.0.7 fixes two security issues and several bugs in 3.0.6.
CVE-2020-13254: Potential data leakage via malformed memcached keys
===================================================================
In cases where a memcached backend does not perform key validation, passing
malformed cache keys could result in a key collision, and potential data
leakage. In order to avoid this vulnerability, key validation is added to the
memcached cache backends.
CVE-2020-13596: Possible XSS via admin ``ForeignKeyRawIdWidget``
================================================================

41
tests/cache/tests.py vendored
View File

@ -16,7 +16,7 @@ from unittest import mock
from django.conf import settings
from django.core import management, signals
from django.core.cache import (
DEFAULT_CACHE_ALIAS, CacheKeyWarning, cache, caches,
DEFAULT_CACHE_ALIAS, CacheKeyWarning, InvalidCacheKey, cache, caches,
)
from django.core.cache.backends.base import InvalidCacheBackendError
from django.core.cache.utils import make_template_fragment_key
@ -623,10 +623,10 @@ class BaseCacheTests:
def _perform_invalid_key_test(self, key, expected_warning):
"""
All the builtin backends (except memcached, see below) should warn on
keys that would be refused by memcached. This encourages portable
caching code without making it too difficult to use production backends
with more liberal key rules. Refs #6447.
All the builtin backends should warn (except memcached that should
error) on keys that would be refused by memcached. This encourages
portable caching code without making it too difficult to use production
backends with more liberal key rules. Refs #6447.
"""
# mimic custom ``make_key`` method being defined since the default will
# never show the below warnings
@ -1270,24 +1270,14 @@ class BaseMemcachedTests(BaseCacheTests):
with self.settings(CACHES={'default': params}):
self.assertEqual(cache._servers, ['server1.tld', 'server2:11211'])
def test_invalid_key_characters(self):
def _perform_invalid_key_test(self, key, expected_warning):
"""
On memcached, we don't introduce a duplicate key validation
step (for speed reasons), we just let the memcached API
library raise its own exception on bad keys. Refs #6447.
In order to be memcached-API-library agnostic, we only assert
that a generic exception of some kind is raised.
Whilst other backends merely warn, memcached should raise for an
invalid key.
"""
# memcached does not allow whitespace or control characters in keys
# when using the ascii protocol.
with self.assertRaises(Exception):
cache.set('key with spaces', 'value')
def test_invalid_key_length(self):
# memcached limits key length to 250
with self.assertRaises(Exception):
cache.set('a' * 251, 'value')
msg = expected_warning.replace(key, ':1:%s' % key)
with self.assertRaisesMessage(InvalidCacheKey, msg):
cache.set(key, 'value')
def test_default_never_expiring_timeout(self):
# Regression test for #22845
@ -1396,15 +1386,6 @@ class PyLibMCCacheTests(BaseMemcachedTests, TestCase):
# libmemcached manages its own connections.
should_disconnect_on_close = False
# By default, pylibmc/libmemcached don't verify keys client-side and so
# this test triggers a server-side bug that causes later tests to fail
# (#19914). The `verify_keys` behavior option could be set to True (which
# would avoid triggering the server-side bug), however this test would
# still fail due to https://github.com/lericson/pylibmc/issues/219.
@unittest.skip("triggers a memcached-server bug, causing subsequent tests to fail")
def test_invalid_key_characters(self):
pass
@override_settings(CACHES=caches_setting_for_tests(
base=PyLibMCCache_params,
exclude=memcached_excluded_caches,