티스토리 수익 글 보기

티스토리 수익 글 보기

[2.2.x] Fixed CVE-2020-13254 — Enforced cache key validation in memc… · django/django@07e59ca · GitHub
Skip to content

Commit 07e59ca

Browse files
danpalmercarltongibson
danpalmer
authored and
carltongibson
committed
[2.2.x] Fixed CVE-2020-13254 — Enforced cache key validation in memcached backends.
1 parent 6d61860 commit 07e59ca

File tree

5 files changed

+58
–45
lines changed

5 files changed

+58
–45
lines changed

‎django/core/cache/__init__.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,13 @@
1717
from django.conf import settings
1818
from django.core import signals
1919
from django.core.cache.backends.base import (
20-
BaseCache, CacheKeyWarning, InvalidCacheBackendError,
20+
BaseCache, CacheKeyWarning, InvalidCacheBackendError, InvalidCacheKey,
2121
)
2222
from django.utils.module_loading import import_string
2323

2424
__all__ = [
2525
'cache', 'caches', 'DEFAULT_CACHE_ALIAS', 'InvalidCacheBackendError',
26-
'CacheKeyWarning', 'BaseCache',
26+
'CacheKeyWarning', 'BaseCache', 'InvalidCacheKey',
2727
]
2828

2929
DEFAULT_CACHE_ALIAS = 'default'

‎django/core/cache/backends/base.py‎

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ class CacheKeyWarning(RuntimeWarning):
1414
pass
1515

1616

17+
class InvalidCacheKey(ValueError):
18+
pass
19+
20+
1721
# Stub class to ensure not passing in a `timeout` argument results in
1822
# the default timeout
1923
DEFAULT_TIMEOUT = object()
@@ -242,18 +246,8 @@ def validate_key(self, key):
242246
backend. This encourages (but does not force) writing backend-portable
243247
cache code.
244248
"""
245-
if len(key) > MEMCACHE_MAX_KEY_LENGTH:
246-
warnings.warn(
247-
'Cache key will cause errors if used with memcached: %r '
248-
'(longer than %s)' % (key, MEMCACHE_MAX_KEY_LENGTH), CacheKeyWarning
249-
)
250-
for char in key:
251-
if ord(char) < 33 or ord(char) == 127:
252-
warnings.warn(
253-
'Cache key contains characters that will cause errors if '
254-
'used with memcached: %r' % key, CacheKeyWarning
255-
)
256-
break
249+
for warning in memcache_key_warnings(key):
250+
warnings.warn(warning, CacheKeyWarning)
257251

258252
def incr_version(self, key, delta=1, version=None):
259253
"""
@@ -281,3 +275,18 @@ def decr_version(self, key, delta=1, version=None):
281275
def close(self, **kwargs):
282276
"""Close the cache connection"""
283277
pass
278+
279+
280+
def memcache_key_warnings(key):
281+
if len(key) > MEMCACHE_MAX_KEY_LENGTH:
282+
yield (
283+
'Cache key will cause errors if used with memcached: %r '
284+
'(longer than %s)' % (key, MEMCACHE_MAX_KEY_LENGTH)
285+
)
286+
for char in key:
287+
if ord(char) < 33 or ord(char) == 127:
288+
yield (
289+
'Cache key contains characters that will cause errors if '
290+
'used with memcached: %r' % key, CacheKeyWarning
291+
)
292+
break

‎django/core/cache/backends/memcached.py‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
import re
55
import time
66

7-
from django.core.cache.backends.base import DEFAULT_TIMEOUT, BaseCache
7+
from django.core.cache.backends.base import (
8+
DEFAULT_TIMEOUT, BaseCache, InvalidCacheKey, memcache_key_warnings,
9+
)
810
from django.utils.functional import cached_property
911

1012

@@ -64,27 +66,33 @@ def get_backend_timeout(self, timeout=DEFAULT_TIMEOUT):
6466

6567
def add(self, key, value, timeout=DEFAULT_TIMEOUT, version=None):
6668
key = self.make_key(key, version=version)
69+
self.validate_key(key)
6770
return self._cache.add(key, value, self.get_backend_timeout(timeout))
6871

6972
def get(self, key, default=None, version=None):
7073
key = self.make_key(key, version=version)
74+
self.validate_key(key)
7175
val = self._cache.get(key)
7276
if val is None:
7377
return default
7478
return val
7579

7680
def set(self, key, value, timeout=DEFAULT_TIMEOUT, version=None):
7781
key = self.make_key(key, version=version)
82+
self.validate_key(key)
7883
if not self._cache.set(key, value, self.get_backend_timeout(timeout)):
7984
# make sure the key doesn't keep its old value in case of failure to set (memcached's 1MB limit)
8085
self._cache.delete(key)
8186

8287
def delete(self, key, version=None):
8388
key = self.make_key(key, version=version)
89+
self.validate_key(key)
8490
self._cache.delete(key)
8591

8692
def get_many(self, keys, version=None):
8793
key_map = {self.make_key(key, version=version): key for key in keys}
94+
for key in key_map:
95+
self.validate_key(key)
8896
ret = self._cache.get_multi(key_map.keys())
8997
return {key_map[k]: v for k, v in ret.items()}
9098

@@ -94,6 +102,7 @@ def close(self, **kwargs):
94102

95103
def incr(self, key, delta=1, version=None):
96104
key = self.make_key(key, version=version)
105+
self.validate_key(key)
97106
# memcached doesn't support a negative delta
98107
if delta < 0:
99108
return self._cache.decr(key, -delta)
@@ -112,6 +121,7 @@ def incr(self, key, delta=1, version=None):
112121

113122
def decr(self, key, delta=1, version=None):
114123
key = self.make_key(key, version=version)
124+
self.validate_key(key)
115125
# memcached doesn't support a negative delta
116126
if delta < 0:
117127
return self._cache.incr(key, -delta)
@@ -133,6 +143,7 @@ def set_many(self, data, timeout=DEFAULT_TIMEOUT, version=None):
133143
original_keys = {}
134144
for key, value in data.items():
135145
safe_key = self.make_key(key, version=version)
146+
self.validate_key(safe_key)
136147
safe_data[safe_key] = value
137148
original_keys[safe_key] = key
138149
failed_keys = self._cache.set_multi(safe_data, self.get_backend_timeout(timeout))
@@ -144,6 +155,10 @@ def delete_many(self, keys, version=None):
144155
def clear(self):
145156
self._cache.flush_all()
146157

158+
def validate_key(self, key):
159+
for warning in memcache_key_warnings(key):
160+
raise InvalidCacheKey(warning)
161+
147162

148163
class MemcachedCache(BaseMemcachedCache):
149164
"An implementation of a cache binding using python-memcached"

‎docs/releases/2.2.13.txt‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,14 @@ Django 2.2.13 release notes
66

77
Django 2.2.13 fixes two security issues and a regression in 2.2.12.
88

9+
CVE-2020-13254: Potential data leakage via malformed memcached keys
10+
===================================================================
11+
12+
In cases where a memcached backend does not perform key validation, passing
13+
malformed cache keys could result in a key collision, and potential data
14+
leakage. In order to avoid this vulnerability, key validation is added to the
15+
memcached cache backends.
16+
917
CVE-2020-13596: Possible XSS via admin ``ForeignKeyRawIdWidget``
1018
================================================================
1119

‎tests/cache/tests.py‎

Lines changed: 11 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
from django.conf import settings
1616
from django.core import management, signals
1717
from django.core.cache import (
18-
DEFAULT_CACHE_ALIAS, CacheKeyWarning, cache, caches,
18+
DEFAULT_CACHE_ALIAS, CacheKeyWarning, InvalidCacheKey, cache, caches,
1919
)
2020
from django.core.cache.utils import make_template_fragment_key
2121
from django.db import close_old_connections, connection, connections
@@ -605,10 +605,10 @@ def test_zero_cull(self):
605605

606606
def _perform_invalid_key_test(self, key, expected_warning):
607607
"""
608-
All the builtin backends (except memcached, see below) should warn on
609-
keys that would be refused by memcached. This encourages portable
610-
caching code without making it too difficult to use production backends
611-
with more liberal key rules. Refs #6447.
608+
All the builtin backends should warn (except memcached that should
609+
error) on keys that would be refused by memcached. This encourages
610+
portable caching code without making it too difficult to use production
611+
backends with more liberal key rules. Refs #6447.
612612
"""
613613
# mimic custom ``make_key`` method being defined since the default will
614614
# never show the below warnings
@@ -1251,24 +1251,14 @@ def test_location_multiple_servers(self):
12511251
with self.settings(CACHES={'default': params}):
12521252
self.assertEqual(cache._servers, ['server1.tld', 'server2:11211'])
12531253

1254-
def test_invalid_key_characters(self):
1254+
def _perform_invalid_key_test(self, key, expected_warning):
12551255
"""
1256-
On memcached, we don't introduce a duplicate key validation
1257-
step (for speed reasons), we just let the memcached API
1258-
library raise its own exception on bad keys. Refs #6447.
1259-
1260-
In order to be memcached-API-library agnostic, we only assert
1261-
that a generic exception of some kind is raised.
1256+
Whilst other backends merely warn, memcached should raise for an
1257+
invalid key.
12621258
"""
1263-
# memcached does not allow whitespace or control characters in keys
1264-
# when using the ascii protocol.
1265-
with self.assertRaises(Exception):
1266-
cache.set('key with spaces', 'value')
1267-
1268-
def test_invalid_key_length(self):
1269-
# memcached limits key length to 250
1270-
with self.assertRaises(Exception):
1271-
cache.set('a' * 251, 'value')
1259+
msg = expected_warning.replace(key, ':1:%s' % key)
1260+
with self.assertRaisesMessage(InvalidCacheKey, msg):
1261+
cache.set(key, 'value')
12721262

12731263
def test_default_never_expiring_timeout(self):
12741264
# Regression test for #22845
@@ -1377,15 +1367,6 @@ class PyLibMCCacheTests(BaseMemcachedTests, TestCase):
13771367
# libmemcached manages its own connections.
13781368
should_disconnect_on_close = False
13791369

1380-
# By default, pylibmc/libmemcached don't verify keys client-side and so
1381-
# this test triggers a server-side bug that causes later tests to fail
1382-
# (#19914). The `verify_keys` behavior option could be set to True (which
1383-
# would avoid triggering the server-side bug), however this test would
1384-
# still fail due to https://github.com/lericson/pylibmc/issues/219.
1385-
@unittest.skip("triggers a memcached-server bug, causing subsequent tests to fail")
1386-
def test_invalid_key_characters(self):
1387-
pass
1388-
13891370
@override_settings(CACHES=caches_setting_for_tests(
13901371
base=PyLibMCCache_params,
13911372
exclude=memcached_excluded_caches,

0 commit comments

Comments
 (0)