File CVE-2024-39330.patch of Package python-Django

Overview Repositories Revisions Requests Users Attributes Meta

File CVE-2024-39330.patch of Package python-Django (Revision 31a1285e30f8c5ca557c9867ce0d1b54)

Currently displaying revision 31a1285e30f8c5ca557c9867ce0d1b54 , Show latest

From ad99364a03b2857f8b694b7d8610d8eefbe78b4d Mon Sep 17 00:00:00 2001
From: nkrapp <nico.krapp@suse.com>
Date: Fri, 26 Jul 2024 15:19:08 +0200
Subject: [PATCH] Fixed CVE-2024-39330 -- Added extra file name
 validation in  Storage's save method.

---
 django/core/files/storage.py    | 16 ++++++++-
 django/core/files/utils.py      | 24 +++++++++++++
 tests/file_storage/test_base.py | 64 +++++++++++++++++++++++++++++++++
 4 files changed, 103 insertions(+), 1 deletions(-)
 create mode 100644 tests/file_storage/test_base.py

diff --git a/django/core/files/storage.py b/django/core/files/storage.py
index 30788d6d75..ec35252a26 100644
--- a/django/core/files/storage.py
+++ b/django/core/files/storage.py
@@ -45,8 +45,22 @@ class Storage:
         if not hasattr(content, 'chunks'):
             content = File(content, name)
 
+        # Ensure that the name is valid, before and after having the storage
+        # system potentially modifying the name. This duplicates the check made
+        # inside `get_available_name` but it's necessary for those cases where
+        # `get_available_name` is overriden and validation is lost.
+        validate_file_name(name, allow_relative_path=True)
+
+        # Potentially find a different name depending on storage constraints.
         name = self.get_available_name(name, max_length=max_length)
-        return self._save(name, content)
+        # Validate the (potentially) new name.
+        validate_file_name(name, allow_relative_path=True)
+
+        # The save operation should return the actual name of the file saved.
+        name = self._save(name, content)
+        # Ensure that the name returned from the storage system is still valid.
+        validate_file_name(name, allow_relative_path=True)
+        return name
 
     # These methods are part of the public API, with default implementations.
 
diff --git a/django/core/files/utils.py b/django/core/files/utils.py
index de89607175..98f05f16ad 100644
--- a/django/core/files/utils.py
+++ b/django/core/files/utils.py
@@ -1,3 +1,27 @@
+import os
+import pathlib
+
+from django.core.exceptions import SuspiciousFileOperation
+
+
+def validate_file_name(name, allow_relative_path=False):
+    # Remove potentially dangerous names
+    if os.path.basename(name) in {"", ".", ".."}:
+        raise SuspiciousFileOperation("Could not derive file name from '%s'" % name)
+
+    if allow_relative_path:
+        # Ensure that name can be treated as a pure posix path, i.e. Unix
+        # style (with forward slashes).
+        path = pathlib.PurePosixPath(str(name).replace("\\", "/"))
+        if path.is_absolute() or ".." in path.parts:
+            raise SuspiciousFileOperation(
+                "Detected path traversal attempt in '%s'" % name
+            )
+    elif name != os.path.basename(name):
+        raise SuspiciousFileOperation("File name '%s' includes path elements" % name)
+    return name
+
+
 class FileProxyMixin:
     """
     A mixin class used to forward file methods to an underlaying file
diff --git a/tests/file_storage/test_base.py b/tests/file_storage/test_base.py
new file mode 100644
index 0000000000..7a0838f7a5
--- /dev/null
+++ b/tests/file_storage/test_base.py
@@ -0,0 +1,64 @@
+import os
+from unittest import mock
+
+from django.core.exceptions import SuspiciousFileOperation
+from django.core.files.storage import Storage
+from django.test import SimpleTestCase
+
+
+class CustomStorage(Storage):
+    """Simple Storage subclass implementing the bare minimum for testing."""
+
+    def exists(self, name):
+        return False
+
+    def _save(self, name):
+        return name
+
+
+class StorageValidateFileNameTests(SimpleTestCase):
+    invalid_file_names = [
+        os.path.join("path", "to", os.pardir, "test.file"),
+        os.path.join(os.path.sep, "path", "to", "test.file"),
+    ]
+    error_msg = "Detected path traversal attempt in '%s'"
+
+    def test_validate_before_get_available_name(self):
+        s = CustomStorage()
+        # The initial name passed to `save` is not valid nor safe, fail early.
+        for name in self.invalid_file_names:
+            with self.subTest(name=name):
+                with mock.patch.object(s, "get_available_name") as mock_get_available_names:
+                    with mock.patch.object(s, "_save") as mock_internal_save:
+                        with self.assertRaisesMessage(
+                            SuspiciousFileOperation, self.error_msg % name
+                        ):
+                            s.save(name, content="irrelevant")
+                        self.assertEqual(mock_get_available_names.mock_calls, [])
+                        self.assertEqual(mock_internal_save.mock_calls, [])
+
+    def test_validate_after_get_available_name(self):
+        s = CustomStorage()
+        # The initial name passed to `save` is valid and safe, but the returned
+        # name from `get_available_name` is not.
+        for name in self.invalid_file_names:
+            with self.subTest(name=name):
+                with mock.patch.object(s, "get_available_name", return_value=name):
+                    with mock.patch.object(s, "_save") as mock_internal_save:
+                        with self.assertRaisesMessage(
+                            SuspiciousFileOperation, self.error_msg % name
+                        ):
+                            s.save("valid-file-name.txt", content="irrelevant")
+                        self.assertEqual(mock_internal_save.mock_calls, [])
+
+    def test_validate_after_internal_save(self):
+        s = CustomStorage()
+        # The initial name passed to `save` is valid and safe, but the result
+        # from `_save` is not (this is achieved by monkeypatching _save).
+        for name in self.invalid_file_names:
+            with self.subTest(name=name):
+                with mock.patch.object(s, "_save", return_value=name):
+                    with self.assertRaisesMessage(
+                        SuspiciousFileOperation, self.error_msg % name
+                    ):
+                        s.save("valid-file-name.txt", content="irrelevant")
-- 
2.45.2

Places

File CVE-2024-39330.patch of Package python-Django (Revision 31a1285e30f8c5ca557c9867ce0d1b54)

Places