Browse Source

Fixed #31983 -- Added system check for file system caches location.

Thanks Johannes Maron and Nick Pope for reviews.
christa 4 years ago
parent
commit
c36075ac1d
4 changed files with 117 additions and 3 deletions
  1. 41 2
      django/core/checks/caches.py
  2. 4 0
      docs/ref/checks.txt
  3. 10 0
      docs/topics/cache.txt
  4. 62 1
      tests/check_framework/test_caches.py

+ 41 - 2
django/core/checks/caches.py

@@ -1,7 +1,10 @@
+import pathlib
+
 from django.conf import settings
-from django.core.cache import DEFAULT_CACHE_ALIAS
+from django.core.cache import DEFAULT_CACHE_ALIAS, caches
+from django.core.cache.backends.filebased import FileBasedCache
 
-from . import Error, Tags, register
+from . import Error, Tags, Warning, register
 
 E001 = Error(
     "You must define a '%s' cache in your CACHES setting." % DEFAULT_CACHE_ALIAS,
@@ -14,3 +17,39 @@ def check_default_cache_is_configured(app_configs, **kwargs):
     if DEFAULT_CACHE_ALIAS not in settings.CACHES:
         return [E001]
     return []
+
+
+@register(Tags.caches, deploy=True)
+def check_cache_location_not_exposed(app_configs, **kwargs):
+    errors = []
+    for name in ('MEDIA_ROOT', 'STATIC_ROOT', 'STATICFILES_DIRS'):
+        setting = getattr(settings, name, None)
+        if not setting:
+            continue
+        if name == 'STATICFILES_DIRS':
+            paths = {
+                pathlib.Path(staticfiles_dir).resolve()
+                for staticfiles_dir in setting
+            }
+        else:
+            paths = {pathlib.Path(setting).resolve()}
+        for alias in settings.CACHES:
+            cache = caches[alias]
+            if not isinstance(cache, FileBasedCache):
+                continue
+            cache_path = pathlib.Path(cache._dir).resolve()
+            if any(path == cache_path for path in paths):
+                relation = 'matches'
+            elif any(path in cache_path.parents for path in paths):
+                relation = 'is inside'
+            elif any(cache_path in path.parents for path in paths):
+                relation = 'contains'
+            else:
+                continue
+            errors.append(Warning(
+                f"Your '{alias}' cache configuration might expose your cache "
+                f"or lead to corruption of your data because its LOCATION "
+                f"{relation} {name}.",
+                id='caches.W002',
+            ))
+    return errors

+ 4 - 0
docs/ref/checks.txt

@@ -138,6 +138,10 @@ configured:
 
 * **caches.E001**: You must define a ``'default'`` cache in your
   :setting:`CACHES` setting.
+* **caches.W002**: Your ``<cache>`` configuration might expose your cache or
+  lead to corruption of your data because its
+  :setting:`LOCATION <CACHES-LOCATION>` matches/is inside/contains
+  :setting:`MEDIA_ROOT`/:setting:`STATIC_ROOT`/:setting:`STATICFILES_DIRS`.
 
 Database
 --------

+ 10 - 0
docs/topics/cache.txt

@@ -293,6 +293,16 @@ above example, if your server runs as the user ``apache``, make sure the
 directory ``/var/tmp/django_cache`` exists and is readable and writable by the
 user ``apache``.
 
+.. warning::
+
+    When the cache :setting:`LOCATION <CACHES-LOCATION>` is contained within
+    :setting:`MEDIA_ROOT`, :setting:`STATIC_ROOT`, or
+    :setting:`STATICFILES_FINDERS`, sensitive data may be exposed.
+
+    An attacker who gains access to the cache file can not only falsify HTML
+    content, which your site will trust, but also remotely execute arbitrary
+    code, as the data is serialized using :mod:`pickle`.
+
 .. _local-memory-caching:
 
 Local-memory caching

+ 62 - 1
tests/check_framework/test_caches.py

@@ -1,4 +1,9 @@
-from django.core.checks.caches import E001, check_default_cache_is_configured
+import pathlib
+
+from django.core.checks import Warning
+from django.core.checks.caches import (
+    E001, check_cache_location_not_exposed, check_default_cache_is_configured,
+)
 from django.test import SimpleTestCase
 from django.test.utils import override_settings
 
@@ -28,3 +33,59 @@ class CheckCacheSettingsAppDirsTest(SimpleTestCase):
         Error if 'default' not present in CACHES setting.
         """
         self.assertEqual(check_default_cache_is_configured(None), [E001])
+
+
+class CheckCacheLocationTest(SimpleTestCase):
+    warning_message = (
+        "Your 'default' cache configuration might expose your cache or lead "
+        "to corruption of your data because its LOCATION %s %s."
+    )
+
+    @staticmethod
+    def get_settings(setting, cache_path, setting_path):
+        return {
+            'CACHES': {
+                'default': {
+                    'BACKEND': 'django.core.cache.backends.filebased.FileBasedCache',
+                    'LOCATION': cache_path,
+                },
+            },
+            setting: [setting_path] if setting == 'STATICFILES_DIRS' else setting_path,
+        }
+
+    def test_cache_path_matches_media_static_setting(self):
+        root = pathlib.Path.cwd()
+        for setting in ('MEDIA_ROOT', 'STATIC_ROOT', 'STATICFILES_DIRS'):
+            settings = self.get_settings(setting, root, root)
+            with self.subTest(setting=setting), self.settings(**settings):
+                msg = self.warning_message % ('matches', setting)
+                self.assertEqual(check_cache_location_not_exposed(None), [
+                    Warning(msg, id='caches.W002'),
+                ])
+
+    def test_cache_path_inside_media_static_setting(self):
+        root = pathlib.Path.cwd()
+        for setting in ('MEDIA_ROOT', 'STATIC_ROOT', 'STATICFILES_DIRS'):
+            settings = self.get_settings(setting, root / 'cache', root)
+            with self.subTest(setting=setting), self.settings(**settings):
+                msg = self.warning_message % ('is inside', setting)
+                self.assertEqual(check_cache_location_not_exposed(None), [
+                    Warning(msg, id='caches.W002'),
+                ])
+
+    def test_cache_path_contains_media_static_setting(self):
+        root = pathlib.Path.cwd()
+        for setting in ('MEDIA_ROOT', 'STATIC_ROOT', 'STATICFILES_DIRS'):
+            settings = self.get_settings(setting, root, root / 'other')
+            with self.subTest(setting=setting), self.settings(**settings):
+                msg = self.warning_message % ('contains', setting)
+                self.assertEqual(check_cache_location_not_exposed(None), [
+                    Warning(msg, id='caches.W002'),
+                ])
+
+    def test_cache_path_not_conflict(self):
+        root = pathlib.Path.cwd()
+        for setting in ('MEDIA_ROOT', 'STATIC_ROOT', 'STATICFILES_DIRS'):
+            settings = self.get_settings(setting, root / 'cache', root / 'other')
+            with self.subTest(setting=setting), self.settings(**settings):
+                self.assertEqual(check_cache_location_not_exposed(None), [])