mirror of
https://github.com/openembedded/meta-openembedded.git
synced 2026-06-13 17:39:57 +00:00
python3-django: fix CVE-2024-39330
Reference: https://nvd.nist.gov/vuln/detail/CVE-2024-39330 Upstream-patch: https://github.com/django/django/commit/2b00edc0151a660d1eb86da4059904a0fc4e095e Signed-off-by: Saravanan <saravanan.kadambathursubramaniyam@windriver.com> Signed-off-by: Gyorgy Sarvari <skandigraun@gmail.com>
This commit is contained in:
committed by
Gyorgy Sarvari
parent
740980aaba
commit
8b438a9d7b
@@ -0,0 +1,184 @@
|
||||
From 2b00edc0151a660d1eb86da4059904a0fc4e095e Mon Sep 17 00:00:00 2001
|
||||
From: Natalia <124304+nessita@users.noreply.github.com>
|
||||
Date: Wed, 20 Mar 2024 13:55:21 -0300
|
||||
Subject: [PATCH] Fixed CVE-2024-39330 -- Added extra file name validation in
|
||||
Storage's save method.
|
||||
|
||||
Thanks to Josh Schneier for the report, and to Carlton Gibson and Sarah
|
||||
Boyce for the reviews.
|
||||
|
||||
CVE: CVE-2024-39330
|
||||
|
||||
Upstream-Status: Backport
|
||||
https://github.com/django/django/commit/2b00edc0151a660d1eb86da4059904a0fc4e095e
|
||||
|
||||
Signed-off-by: Saravanan <saravanan.kadambathursubramaniyam@windriver.com>
|
||||
---
|
||||
django/core/files/storage.py | 11 ++++++
|
||||
django/core/files/utils.py | 7 ++--
|
||||
docs/releases/3.2.25.txt | 12 ++++++
|
||||
tests/file_storage/test_base.py | 70 +++++++++++++++++++++++++++++++++
|
||||
tests/file_storage/tests.py | 6 ---
|
||||
5 files changed, 96 insertions(+), 10 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 22984f9..680f5ec 100644
|
||||
--- a/django/core/files/storage.py
|
||||
+++ b/django/core/files/storage.py
|
||||
@@ -50,7 +50,18 @@ 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)
|
||||
+ # 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)
|
||||
diff --git a/django/core/files/utils.py b/django/core/files/utils.py
|
||||
index f28cea1..a1fea44 100644
|
||||
--- a/django/core/files/utils.py
|
||||
+++ b/django/core/files/utils.py
|
||||
@@ -10,10 +10,9 @@ def validate_file_name(name, allow_relative_path=False):
|
||||
raise SuspiciousFileOperation("Could not derive file name from '%s'" % name)
|
||||
|
||||
if allow_relative_path:
|
||||
- # Use PurePosixPath() because this branch is checked only in
|
||||
- # FileField.generate_filename() where all file paths are expected to be
|
||||
- # Unix style (with forward slashes).
|
||||
- path = pathlib.PurePosixPath(name)
|
||||
+ # 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
|
||||
diff --git a/docs/releases/3.2.25.txt b/docs/releases/3.2.25.txt
|
||||
index a613b08..60236d5 100644
|
||||
--- a/docs/releases/3.2.25.txt
|
||||
+++ b/docs/releases/3.2.25.txt
|
||||
@@ -47,6 +47,18 @@ The :meth:`~django.contrib.auth.backends.ModelBackend.authenticate()` method
|
||||
allowed remote attackers to enumerate users via a timing attack involving login
|
||||
requests for users with unusable passwords.
|
||||
|
||||
+CVE-2024-39330: Potential directory-traversal via ``Storage.save()``
|
||||
+====================================================================
|
||||
+
|
||||
+Derived classes of the :class:`~django.core.files.storage.Storage` base class
|
||||
+which override :meth:`generate_filename()
|
||||
+<django.core.files.storage.Storage.generate_filename()>` without replicating
|
||||
+the file path validations existing in the parent class, allowed for potential
|
||||
+directory-traversal via certain inputs when calling :meth:`save()
|
||||
+<django.core.files.storage.Storage.save()>`.
|
||||
+
|
||||
+Built-in ``Storage`` sub-classes were not affected by this vulnerability.
|
||||
+
|
||||
Bugfixes
|
||||
========
|
||||
|
||||
diff --git a/tests/file_storage/test_base.py b/tests/file_storage/test_base.py
|
||||
new file mode 100644
|
||||
index 0000000..c5338b8
|
||||
--- /dev/null
|
||||
+++ b/tests/file_storage/test_base.py
|
||||
@@ -0,0 +1,70 @@
|
||||
+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),
|
||||
+ mock.patch.object(s, "get_available_name") as mock_get_available_name,
|
||||
+ 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_name.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),
|
||||
+ mock.patch.object(s, "get_available_name", return_value=name),
|
||||
+ 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),
|
||||
+ mock.patch.object(s, "_save", return_value=name),
|
||||
+ ):
|
||||
+ with self.assertRaisesMessage(
|
||||
+ SuspiciousFileOperation, self.error_msg % name
|
||||
+ ):
|
||||
+ s.save("valid-file-name.txt", content="irrelevant")
|
||||
diff --git a/tests/file_storage/tests.py b/tests/file_storage/tests.py
|
||||
index 7238093..6d17a71 100644
|
||||
--- a/tests/file_storage/tests.py
|
||||
+++ b/tests/file_storage/tests.py
|
||||
@@ -297,12 +297,6 @@ class FileStorageTests(SimpleTestCase):
|
||||
|
||||
self.storage.delete('path/to/test.file')
|
||||
|
||||
- def test_file_save_abs_path(self):
|
||||
- test_name = 'path/to/test.file'
|
||||
- f = ContentFile('file saved with path')
|
||||
- f_name = self.storage.save(os.path.join(self.temp_dir, test_name), f)
|
||||
- self.assertEqual(f_name, test_name)
|
||||
-
|
||||
def test_save_doesnt_close(self):
|
||||
with TemporaryUploadedFile('test', 'text/plain', 1, 'utf8') as file:
|
||||
file.write(b'1')
|
||||
--
|
||||
2.48.1
|
||||
|
||||
@@ -0,0 +1,181 @@
|
||||
From 2b00edc0151a660d1eb86da4059904a0fc4e095e Mon Sep 17 00:00:00 2001
|
||||
From: Natalia <124304+nessita@users.noreply.github.com>
|
||||
Date: Wed, 20 Mar 2024 13:55:21 -0300
|
||||
Subject: [PATCH] Fixed CVE-2024-39330 -- Added extra file name validation in
|
||||
Storage's save method.
|
||||
|
||||
Thanks to Josh Schneier for the report, and to Carlton Gibson and Sarah
|
||||
Boyce for the reviews.
|
||||
|
||||
CVE: CVE-2024-39330
|
||||
|
||||
Upstream-Status: Backport
|
||||
https://github.com/django/django/commit/2b00edc0151a660d1eb86da4059904a0fc4e095e
|
||||
|
||||
Signed-off-by: Saravanan <saravanan.kadambathursubramaniyam@windriver.com>
|
||||
---
|
||||
django/core/files/storage.py | 11 ++++++
|
||||
django/core/files/utils.py | 7 ++--
|
||||
docs/releases/2.2.28.txt | 12 ++++++
|
||||
tests/file_storage/test_base.py | 70 +++++++++++++++++++++++++++++++++
|
||||
tests/file_storage/tests.py | 6 ---
|
||||
5 files changed, 96 insertions(+), 10 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 ea5bbc8..8c633ec 100644
|
||||
--- a/django/core/files/storage.py
|
||||
+++ b/django/core/files/storage.py
|
||||
@@ -50,7 +50,18 @@ 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)
|
||||
+ # 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)
|
||||
diff --git a/django/core/files/utils.py b/django/core/files/utils.py
|
||||
index f28cea1..a1fea44 100644
|
||||
--- a/django/core/files/utils.py
|
||||
+++ b/django/core/files/utils.py
|
||||
@@ -10,10 +10,9 @@ def validate_file_name(name, allow_relative_path=False):
|
||||
raise SuspiciousFileOperation("Could not derive file name from '%s'" % name)
|
||||
|
||||
if allow_relative_path:
|
||||
- # Use PurePosixPath() because this branch is checked only in
|
||||
- # FileField.generate_filename() where all file paths are expected to be
|
||||
- # Unix style (with forward slashes).
|
||||
- path = pathlib.PurePosixPath(name)
|
||||
+ # 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
|
||||
diff --git a/docs/releases/2.2.28.txt b/docs/releases/2.2.28.txt
|
||||
index 22fa80e..3503f38 100644
|
||||
--- a/docs/releases/2.2.28.txt
|
||||
+++ b/docs/releases/2.2.28.txt
|
||||
@@ -131,3 +131,15 @@ The :meth:`~django.contrib.auth.backends.ModelBackend.authenticate()` method
|
||||
allowed remote attackers to enumerate users via a timing attack involving login
|
||||
requests for users with unusable passwords.
|
||||
|
||||
+CVE-2024-39330: Potential directory-traversal via ``Storage.save()``
|
||||
+====================================================================
|
||||
+
|
||||
+Derived classes of the :class:`~django.core.files.storage.Storage` base class
|
||||
+which override :meth:`generate_filename()
|
||||
+<django.core.files.storage.Storage.generate_filename()>` without replicating
|
||||
+the file path validations existing in the parent class, allowed for potential
|
||||
+directory-traversal via certain inputs when calling :meth:`save()
|
||||
+<django.core.files.storage.Storage.save()>`.
|
||||
+
|
||||
+Built-in ``Storage`` sub-classes were not affected by this vulnerability.
|
||||
+
|
||||
diff --git a/tests/file_storage/test_base.py b/tests/file_storage/test_base.py
|
||||
new file mode 100644
|
||||
index 0000000..c5338b8
|
||||
--- /dev/null
|
||||
+++ b/tests/file_storage/test_base.py
|
||||
@@ -0,0 +1,70 @@
|
||||
+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),
|
||||
+ mock.patch.object(s, "get_available_name") as mock_get_available_name,
|
||||
+ 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_name.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),
|
||||
+ mock.patch.object(s, "get_available_name", return_value=name),
|
||||
+ 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),
|
||||
+ mock.patch.object(s, "_save", return_value=name),
|
||||
+ ):
|
||||
+ with self.assertRaisesMessage(
|
||||
+ SuspiciousFileOperation, self.error_msg % name
|
||||
+ ):
|
||||
+ s.save("valid-file-name.txt", content="irrelevant")
|
||||
diff --git a/tests/file_storage/tests.py b/tests/file_storage/tests.py
|
||||
index 4c6f692..0e69264 100644
|
||||
--- a/tests/file_storage/tests.py
|
||||
+++ b/tests/file_storage/tests.py
|
||||
@@ -291,12 +291,6 @@ class FileStorageTests(SimpleTestCase):
|
||||
|
||||
self.storage.delete('path/to/test.file')
|
||||
|
||||
- def test_file_save_abs_path(self):
|
||||
- test_name = 'path/to/test.file'
|
||||
- f = ContentFile('file saved with path')
|
||||
- f_name = self.storage.save(os.path.join(self.temp_dir, test_name), f)
|
||||
- self.assertEqual(f_name, test_name)
|
||||
-
|
||||
def test_save_doesnt_close(self):
|
||||
with TemporaryUploadedFile('test', 'text/plain', 1, 'utf8') as file:
|
||||
file.write(b'1')
|
||||
--
|
||||
2.48.1
|
||||
|
||||
@@ -29,6 +29,7 @@ SRC_URI += "file://CVE-2023-31047.patch \
|
||||
file://CVE-2024-56374.patch \
|
||||
file://CVE-2025-57833.patch \
|
||||
file://CVE-2024-39329.patch \
|
||||
file://CVE-2024-39330.patch \
|
||||
"
|
||||
|
||||
SRC_URI[sha256sum] = "0200b657afbf1bc08003845ddda053c7641b9b24951e52acd51f6abda33a7413"
|
||||
|
||||
@@ -11,6 +11,7 @@ SRC_URI += "\
|
||||
file://CVE-2024-56374.patch \
|
||||
file://CVE-2025-57833.patch \
|
||||
file://CVE-2024-39329.patch \
|
||||
file://CVE-2024-39330.patch \
|
||||
"
|
||||
|
||||
# Set DEFAULT_PREFERENCE so that the LTS version of django is built by
|
||||
|
||||
Reference in New Issue
Block a user