Django security releases 4.0.1, 3.2.11, and 2.2.26 could break the storage check

[cayla]

This could be config specific (our default root etc), but after upgrading to Django 2.2.26, we saw our watchman storage check consistently fail.

See CVE-2021-45452: Potential directory-traversal via Storage.save() on https://www.djangoproject.com/weblog/2022/jan/04/security-releases/ (diff https://github.com/django/django/commit/6d343d01c57eb03ca1c6826318b652709e58a76e)

Example traceback (slightly redacted):

Traceback (most recent call last):
  File "/srv/[redacted]-py3/lib/python3.8/site-packages/watchman/decorators.py", line 28, in wrapped
    response = func(*args, **kwargs)
  File "/srv/[redacted]-py3/lib/python3.8/site-packages/watchman/checks.py", line 63, in _check_storage
    path = default_storage.save(filename, ContentFile(content))
  File "/srv/[redacted]-py3/lib/python3.8/site-packages/django/core/files/storage.py", line 56, in save
    validate_file_name(name, allow_relative_path=True)
  File "/srv/[redacted]-py3/lib/python3.8/site-packages/django/core/files/utils.py", line 18, in validate_file_name
    raise SuspiciousFileOperation(
django.core.exceptions.SuspiciousFileOperation: Detected path traversal attempt in '/usr/local/[redacted]/www/django-watchman-416abbee-8bbb-4f79-98d0-858aa4168824.txt'

[mwarkentin]

@cayla thanks for letting me know about this.. I'll take a look shortly. :)

[mwarkentin]

Possibly related (fix?): https://github.com/mwarkentin/django-watchman/pull/156

[mwarkentin]

@cayla if you need an immediate workaround, you should be able to skip the storage check.

[cayla]

@mwarkentin That is exactly what we are going to do for today. :)

[mwarkentin]

@cayla I just redeployed one of our apps with Django 3.2.11 and our watchman storage check seems to be working:

# pip list | grep Django
Django                            3.2.11

# python manage.py watchman -v3
{"storage": {"ok": true}}
{"databases": [{"default": {"ok": true}}, {"read_replica": {"ok": true}}]}
{"caches": [{"default": {"ok": true}}]}

Are you able to provide some more details / example configuration of your storage? Looks like you're using the built in Django file storage - are you setting your own WATCHMAN_STORAGE_ROOT / MEDIA_ROOT?

[mwarkentin]

I've reproduced in the sample project:

❯ g diff
diff --git a/sample_project/requirements.txt b/sample_project/requirements.txt
index 8a72e2a..78c54a6 100644
--- a/sample_project/requirements.txt
+++ b/sample_project/requirements.txt
@@ -1,4 +1,4 @@
-django<2.0,>=1.11
+django==3.2.11

 # django-watchman
 -e /app
diff --git a/sample_project/sample_project/settings.py b/sample_project/sample_project/settings.py
index b4438d6..0b4f5d7 100644
--- a/sample_project/sample_project/settings.py
+++ b/sample_project/sample_project/settings.py
@@ -31,6 +31,8 @@ ALLOWED_HOSTS = []

 EXPOSE_WATCHMAN_VERSION = True

+WATCHMAN_STORAGE_PATH = "/path_to_your_app/foo/bar/"

{
  "caches": [
    {
      "default": {
        "ok": true
      }
    }
  ],
  "databases": [
    {
      "default": {
        "ok": true
      }
    },
    {
      "secondary": {
        "ok": true
      }
    }
  ],
  "storage": {
    "ok": false,
    "error": "The joined path (/path_to_your_app/foo/bar/django-watchman-0eed31cf-9fb5-4a8f-96bb-166ce24c68f4.txt) is located outside of the base path component (/app/sample_project)",
    "stacktrace": "Traceback (most recent call last):\n  File \"/app/watchman/decorators.py\", line 29, in wrapped\n    response = func(*args, **kwargs)\n  File \"/app/watchman/checks.py\", line 69, in _check_storage\n    path = default_storage.save(filename, ContentFile(content))\n  File \"/usr/local/lib/python3.10/site-packages/django/core/files/storage.py\", line 53, in save\n    name = self.get_available_name(name, max_length=max_length)\n  File \"/usr/local/lib/python3.10/site-packages/django/core/files/storage.py\", line 91, in get_available_name\n    while self.exists(name) or (max_length and len(name) > max_length):\n  File \"/usr/local/lib/python3.10/site-packages/django/core/files/storage.py\", line 325, in exists\n    return os.path.exists(self.path(name))\n  File \"/usr/local/lib/python3.10/site-packages/django/core/files/storage.py\", line 338, in path\n    return safe_join(self.location, name)\n  File \"/usr/local/lib/python3.10/site-packages/django/utils/_os.py\", line 29, in safe_join\n    raise SuspiciousFileOperation(\ndjango.core.exceptions.SuspiciousFileOperation: The joined path (/path_to_your_app/foo/bar/django-watchman-0eed31cf-9fb5-4a8f-96bb-166ce24c68f4.txt) is located outside of the base path component (/app/sample_project)\n"
  }
}

[mwarkentin]

Setting a relative storage path works:

WATCHMAN_STORAGE_PATH = "path_to_your_app/foo/bar/"

@cayla is this something you can work with? Or do you see something that can be done within watchman itself to help with this? Can probably add some pointers in the documentation at a minimum.

[cayla]

@mwarkentin we can definitely work with that. I wish I had explored PR #156 more closely when you linked it before.

For the record, we have a MEDIA_ROOT with an absolute path and WATCHMAN_STORAGE_PATH wasn't set at all.

Thanks for your prompt reply and advice today. Documentation sounds like a good idea in case there are any folks with a setup like ours.

[mwarkentin]

Trying to leave the app root with a relative path (WATCHMAN_STORAGE_PATH = "../../foo/bar") errors as well with the same SuspiciousFileOperation error.