From: Chris Lamb Date: Sat, 13 Jun 2020 15:31:18 +0100 Subject: CVE-2020-13254 --- django/core/cache/__init__.py | 4 ++-- django/core/cache/backends/base.py | 33 +++++++++++++++++++++------------ django/core/cache/backends/memcached.py | 24 ++++++++++++++++++++++-- 3 files changed, 45 insertions(+), 16 deletions(-) diff --git a/django/core/cache/__init__.py b/django/core/cache/__init__.py index 26897ff..dc377a9 100644 --- a/django/core/cache/__init__.py +++ b/django/core/cache/__init__.py @@ -17,13 +17,13 @@ from threading 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', 'DEFAULT_CACHE_ALIAS', 'InvalidCacheBackendError', - 'CacheKeyWarning', 'BaseCache', + 'CacheKeyWarning', 'BaseCache', 'InvalidCacheKey', ] DEFAULT_CACHE_ALIAS = 'default' diff --git a/django/core/cache/backends/base.py b/django/core/cache/backends/base.py index a07a34e..688ffb8 100644 --- a/django/core/cache/backends/base.py +++ b/django/core/cache/backends/base.py @@ -24,6 +24,10 @@ DEFAULT_TIMEOUT = object() MEMCACHE_MAX_KEY_LENGTH = 250 +class InvalidCacheKey(ValueError): + pass + + def default_key_func(key, key_prefix, version): """ Default function to generate keys. @@ -233,18 +237,8 @@ class BaseCache(object): 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): """Adds delta to the cache version for the supplied key. Returns the @@ -270,3 +264,18 @@ class BaseCache(object): 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, + ) + break diff --git a/django/core/cache/backends/memcached.py b/django/core/cache/backends/memcached.py index ee6b3b7..80395e6 100644 --- a/django/core/cache/backends/memcached.py +++ b/django/core/cache/backends/memcached.py @@ -3,7 +3,9 @@ import pickle 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 import six from django.utils.encoding import force_str from django.utils.functional import cached_property @@ -69,10 +71,12 @@ 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) val = self._cache.get(key) if val is None: return default @@ -80,16 +84,20 @@ class BaseMemcachedCache(BaseCache): 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) self._cache.delete(key) def get_many(self, keys, version=None): new_keys = [self.make_key(x, version=version) for x in keys] + for key in new_keys: + self.validate_key(key) ret = self._cache.get_multi(new_keys) if ret: _ = {} @@ -104,6 +112,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) @@ -122,6 +131,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) @@ -142,15 +152,25 @@ class BaseMemcachedCache(BaseCache): safe_data = {} for key, value in data.items(): key = self.make_key(key, version=version) + self.validate_key(key) safe_data[key] = value self._cache.set_multi(safe_data, self.get_backend_timeout(timeout)) def delete_many(self, keys, version=None): - self._cache.delete_multi(self.make_key(key, version=version) for key in keys) + to_delete = [] + for key in to_delete: + key = self.make_key(key, version=version) + self.validate_key(key) + to_delete.append(key) + self._cache.delete_multi(to_delete) 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"