File CVE-2021-31542.patch of Package python-Django

Overview Repositories Revisions Requests Users Attributes Meta

File CVE-2021-31542.patch of Package python-Django

commit c2c3c23bdeacea4d5f78892f94270068870d6a54
Author: Florian Apolloner <florian@apolloner.eu>
Date:   Wed Apr 14 18:23:44 2021 +0200

[2.2.x] Fixed CVE-2021-31542 -- Tightened path & file name sanitation in file uploads.
    
    (cherry picked from commit 04ac1624bdc2fa737188401757cf95ced122d26d)

diff --git a/django/core/files/storage.py b/django/core/files/storage.py
index 98c89ddcfa96..003de8c0b852 100644
--- a/django/core/files/storage.py
+++ b/django/core/files/storage.py
@@ -1,5 +1,6 @@
 import errno
 import os
+import pathlib
 import warnings
 from datetime import datetime
 
@@ -7,6 +8,7 @@ from django.conf import settings
 from django.core.exceptions import SuspiciousFileOperation
 from django.core.files import File, locks
 from django.core.files.move import file_move_safe
+from django.core.files.utils import validate_file_name
 from django.core.signals import setting_changed
 from django.utils import timezone
 from django.utils._os import abspathu, safe_join
@@ -68,6 +70,9 @@ class Storage(object):
         available for new content to be written to.
         """
         dir_name, file_name = os.path.split(name)
+        if '..' in pathlib.PurePath(dir_name).parts:
+            raise SuspiciousFileOperation("Detected path traversal attempt in '%s'" % dir_name)
+        validate_file_name(file_name)
         file_root, file_ext = os.path.splitext(file_name)
         # If the filename already exists, add an underscore and a random 7
         # character alphanumeric string (before the file extension, if one
@@ -100,6 +105,8 @@ class Storage(object):
         """
         # `filename` may include a path as returned by FileField.upload_to.
         dirname, filename = os.path.split(filename)
+        if '..' in pathlib.PurePath(dirname).parts:
+            raise SuspiciousFileOperation("Detected path traversal attempt in '%s'" % dirname)
         return os.path.normpath(os.path.join(dirname, self.get_valid_name(filename)))
 
     def path(self, name):
diff --git a/django/core/files/uploadedfile.py b/django/core/files/uploadedfile.py
index 6f71fc3b95ff..bbe9f428131b 100644
--- a/django/core/files/uploadedfile.py
+++ b/django/core/files/uploadedfile.py
@@ -9,6 +9,7 @@ from io import BytesIO
 from django.conf import settings
 from django.core.files import temp as tempfile
 from django.core.files.base import File
+from django.core.files.utils import validate_file_name
 from django.utils.encoding import force_str
 
 __all__ = ('UploadedFile', 'TemporaryUploadedFile', 'InMemoryUploadedFile',
@@ -51,6 +52,8 @@ class UploadedFile(File):
                 ext = ext[:255]
                 name = name[:255 - len(ext)] + ext
 
+            name = validate_file_name(name)
+
         self._name = name
 
     name = property(_get_name, _set_name)
diff --git a/django/core/files/utils.py b/django/core/files/utils.py
index 8e891bf23f8a..63d320e80f4f 100644
--- a/django/core/files/utils.py
+++ b/django/core/files/utils.py
@@ -1,3 +1,19 @@
+import os
+
+from django.core.exceptions import SuspiciousFileOperation
+
+
+def validate_file_name(name):
+    if name != os.path.basename(name):
+        raise SuspiciousFileOperation("File name '%s' includes path elements" % name)
+
+    # Remove potentially dangerous names
+    if name in {'', '.', '..'}:
+        raise SuspiciousFileOperation("Could not derive file name from '%s'" % name)
+
+    return name
+
+
 class FileProxyMixin(object):
     """
     A mixin class used to forward file methods to an underlaying file
diff --git a/django/db/models/fields/files.py b/django/db/models/fields/files.py
index 6906b8f485cc..c9ba01b77040 100644
--- a/django/db/models/fields/files.py
+++ b/django/db/models/fields/files.py
@@ -8,6 +8,7 @@ from django.core import checks
 from django.core.files.base import File
 from django.core.files.images import ImageFile
 from django.core.files.storage import default_storage
+from django.core.files.utils import validate_file_name
 from django.db.models import signals
 from django.db.models.fields import Field
 from django.utils import six
@@ -323,6 +324,7 @@ class FileField(Field):
         Until the storage layer, all file paths are expected to be Unix style
         (with forward slashes).
         """
+        filename = validate_file_name(filename)
         if callable(self.upload_to):
             filename = self.upload_to(instance, filename)
         else:
diff --git a/django/http/multipartparser.py b/django/http/multipartparser.py
index 2bc9d0e518f6..277120235310 100644
--- a/django/http/multipartparser.py
+++ b/django/http/multipartparser.py
@@ -9,7 +9,7 @@ from __future__ import unicode_literals
 import base64
 import binascii
 import cgi
-import os
+import HTMLParser
 import sys
 
 from django.conf import settings
@@ -23,7 +23,6 @@ from django.utils import six
 from django.utils.datastructures import MultiValueDict
 from django.utils.encoding import force_text
 from django.utils.six.moves.urllib.parse import unquote
-from django.utils.text import unescape_entities
 
 __all__ = ('MultiPartParser', 'MultiPartParserError', 'InputStreamExhausted')
 
@@ -305,10 +304,25 @@ class MultiPartParser(object):
                 break
 
     def sanitize_file_name(self, file_name):
-        file_name = unescape_entities(file_name)
-        # Cleanup Windows-style path separators.
-        file_name = file_name[file_name.rfind('\\') + 1:].strip()
-        return os.path.basename(file_name)
+        """
+        Sanitize the filename of an upload.
+
+        Remove all possible path separators, even though that might remove more
+        than actually required by the target system. Filenames that could
+        potentially cause problems (current/parent dir) are also discarded.
+
+        It should be noted that this function could still return a "filepath"
+        like "C:some_file.txt" which is handled later on by the storage layer.
+        So while this function does sanitize filenames to some extent, the
+        resulting filename should still be considered as untrusted user input.
+        """
+        file_name = HTMLParser.HTMLParser().unescape(file_name)
+        file_name = file_name.rsplit('/')[-1]
+        file_name = file_name.rsplit('\\')[-1]
+
+        if file_name in {'', '.', '..'}:
+            return None
+        return file_name
 
     IE_sanitize = sanitize_file_name
 
diff --git a/django/utils/text.py b/django/utils/text.py
index f221747f6f36..8b138555eeeb 100644
--- a/django/utils/text.py
+++ b/django/utils/text.py
@@ -5,6 +5,7 @@ import unicodedata
 from gzip import GzipFile
 from io import BytesIO
 
+from django.core.exceptions import SuspiciousFileOperation
 from django.utils import six
 from django.utils.encoding import force_text
 from django.utils.functional import (
@@ -234,7 +235,7 @@ class Truncator(SimpleLazyObject):
 
 
 @keep_lazy_text
-def get_valid_filename(s):
+def get_valid_filename(name):
     """
     Returns the given string converted to a string that can be used for a clean
     filename. Specifically, leading and trailing spaces are removed; other
@@ -243,8 +244,11 @@ def get_valid_filename(s):
     >>> get_valid_filename("john's portrait in 2004.jpg")
     'johns_portrait_in_2004.jpg'
     """
-    s = force_text(s).strip().replace(' ', '_')
-    return re.sub(r'(?u)[^-\w.]', '', s)
+    s = str(name).strip().replace(' ', '_')
+    s = re.sub(r'(?u)[^-\w.]', '', s)
+    if s in {'', '.', '..'}:
+        raise SuspiciousFileOperation("Could not derive file name from '%s'" % name)
+    return s
 
 
 @keep_lazy_text
diff --git a/docs/releases/2.2.21.txt b/docs/releases/2.2.21.txt
new file mode 100644
index 000000000000..f32aeadff767
--- /dev/null
+++ b/docs/releases/2.2.21.txt
@@ -0,0 +1,17 @@
+===========================
+Django 2.2.21 release notes
+===========================
+
+*May 4, 2021*
+
+Django 2.2.21 fixes a security issue in 2.2.20.
+
+CVE-2021-31542: Potential directory-traversal via uploaded files
+================================================================
+
+``MultiPartParser``, ``UploadedFile``, and ``FieldFile`` allowed
+directory-traversal via uploaded files with suitably crafted file names.
+
+In order to mitigate this risk, stricter basename and path sanitation is now
+applied. Specifically, empty file names and paths with dot segments will be
+rejected.
diff --git a/docs/releases/index.txt b/docs/releases/index.txt
index be5fb3e54e9a..e59c97b17ff5 100644
--- a/docs/releases/index.txt
+++ b/docs/releases/index.txt
@@ -20,6 +20,75 @@ versions of the documentation contain the release notes for any later releases.
 
 .. _development_release_notes:
 
+2.2 release
+-----------
+.. toctree::
+   :maxdepth: 1
+
+   2.2.21
+   2.2.20
+   2.2.19
+   2.2.18
+   2.2.17
+   2.2.16
+   2.2.15
+   2.2.14
+   2.2.13
+   2.2.12
+   2.2.11
+   2.2.10
+   2.2.9
+   2.2.8
+   2.2.7
+   2.2.6
+   2.2.5
+   2.2.4
+   2.2.3
+   2.2.2
+   2.2.1
+   2.2
+
+2.1 release
+-----------
+.. toctree::
+   :maxdepth: 1
+
+   2.1.15
+   2.1.14
+   2.1.13
+   2.1.12
+   2.1.11
+   2.1.10
+   2.1.9
+   2.1.8
+   2.1.7
+   2.1.6
+   2.1.5
+   2.1.4
+   2.1.3
+   2.1.2
+   2.1.1
+   2.1
+
+2.0 release
+-----------
+.. toctree::
+   :maxdepth: 1
+
+   2.0.13
+   2.0.12
+   2.0.11
+   2.0.10
+   2.0.9
+   2.0.8
+   2.0.7
+   2.0.6
+   2.0.5
+   2.0.4
+   2.0.3
+   2.0.2
+   2.0.1
+   2.0
 
 1.11 release
 ------------
diff --git a/tests/file_storage/test_generate_filename.py b/tests/file_storage/test_generate_filename.py
index 44320138509b..6f79da39278e 100644
--- a/tests/file_storage/test_generate_filename.py
+++ b/tests/file_storage/test_generate_filename.py
@@ -1,8 +1,9 @@
 import os
 import warnings
 
+from django.core.exceptions import SuspiciousFileOperation
 from django.core.files.base import ContentFile
-from django.core.files.storage import Storage
+from django.core.files.storage import FileSystemStorage, Storage
 from django.db.models import FileField
 from django.test import SimpleTestCase
 
@@ -37,6 +38,44 @@ class AWSS3Storage(Storage):
 
 
 class GenerateFilenameStorageTests(SimpleTestCase):
+    def test_storage_dangerous_paths(self):
+        candidates = [
+            ('/tmp/..', '..'),
+            ('/tmp/.', '.'),
+            ('', ''),
+        ]
+        s = FileSystemStorage()
+        msg = "Could not derive file name from '%s'"
+        for file_name, base_name in candidates:
+            with self.subTest(file_name=file_name):
+                with self.assertRaisesMessage(SuspiciousFileOperation, msg % base_name):
+                    s.get_available_name(file_name)
+                with self.assertRaisesMessage(SuspiciousFileOperation, msg % base_name):
+                    s.generate_filename(file_name)
+
+    def test_storage_dangerous_paths_dir_name(self):
+        file_name = '/tmp/../path'
+        s = FileSystemStorage()
+        msg = "Detected path traversal attempt in '/tmp/..'"
+        with self.assertRaisesMessage(SuspiciousFileOperation, msg):
+            s.get_available_name(file_name)
+        with self.assertRaisesMessage(SuspiciousFileOperation, msg):
+            s.generate_filename(file_name)
+
+    def test_filefield_dangerous_filename(self):
+        candidates = ['..', '.', '', '???', '$.$.$']
+        f = FileField(upload_to='some/folder/')
+        msg = "Could not derive file name from '%s'"
+        for file_name in candidates:
+            with self.subTest(file_name=file_name):
+                with self.assertRaisesMessage(SuspiciousFileOperation, msg % file_name):
+                    f.generate_filename(None, file_name)
+
+    def test_filefield_dangerous_filename_dir(self):
+        f = FileField(upload_to='some/folder/')
+        msg = "File name '/tmp/path' includes path elements"
+        with self.assertRaisesMessage(SuspiciousFileOperation, msg):
+            f.generate_filename(None, '/tmp/path')
 
     def test_filefield_get_directory_deprecation(self):
         with warnings.catch_warnings(record=True) as warns:
diff --git a/tests/file_uploads/tests.py b/tests/file_uploads/tests.py
index af7b82126cf6..86a30a6a00fb 100644
--- a/tests/file_uploads/tests.py
+++ b/tests/file_uploads/tests.py
@@ -12,8 +12,9 @@ import tempfile as sys_tempfile
 import unittest
 from io import BytesIO
 
+from django.core.exceptions import SuspiciousFileOperation
 from django.core.files import temp as tempfile
-from django.core.files.uploadedfile import SimpleUploadedFile
+from django.core.files.uploadedfile import SimpleUploadedFile, UploadedFile
 from django.http.multipartparser import MultiPartParser, parse_header
 from django.test import SimpleTestCase, TestCase, client, override_settings
 from django.utils.encoding import force_bytes
@@ -42,6 +43,16 @@ CANDIDATE_TRAVERSAL_FILE_NAMES = [
     '..&#x2F;hax0rd.txt',       # HTML entities.
 ]
 
+CANDIDATE_INVALID_FILE_NAMES = [
+    '/tmp/',        # Directory, *nix-style.
+    'c:\\tmp\\',    # Directory, win-style.
+    '/tmp/.',       # Directory dot, *nix-style.
+    'c:\\tmp\\.',   # Directory dot, *nix-style.
+    '/tmp/..',      # Parent directory, *nix-style.
+    'c:\\tmp\\..',  # Parent directory, win-style.
+    '',             # Empty filename.
+]
+
 
 @override_settings(MEDIA_ROOT=MEDIA_ROOT, ROOT_URLCONF='file_uploads.urls', MIDDLEWARE=[])
 class FileUploadTests(TestCase):
@@ -57,6 +68,22 @@ class FileUploadTests(TestCase):
         shutil.rmtree(MEDIA_ROOT)
         super(FileUploadTests, cls).tearDownClass()
 
+    def test_upload_name_is_validated(self):
+        candidates = [
+            '/tmp/',
+            '/tmp/..',
+            '/tmp/.',
+        ]
+        if sys.platform == 'win32':
+            candidates.extend([
+                'c:\\tmp\\',
+                'c:\\tmp\\..',
+                'c:\\tmp\\.',
+            ])
+        for file_name in candidates:
+            with self.subTest(file_name=file_name):
+                self.assertRaises(SuspiciousFileOperation, UploadedFile, name=file_name)
+
     def test_simple_upload(self):
         with open(__file__, 'rb') as fp:
             post_data = {
@@ -637,6 +664,15 @@ class MultiParserTests(unittest.TestCase):
             with self.subTest(file_name=file_name):
                 self.assertEqual(parser.sanitize_file_name(file_name), 'hax0rd.txt')
 
+    def test_sanitize_invalid_file_name(self):
+        parser = MultiPartParser({
+            'CONTENT_TYPE': 'multipart/form-data; boundary=_foo',
+            'CONTENT_LENGTH': '1',
+        }, StringIO('x'), [], 'utf-8')
+        for file_name in CANDIDATE_INVALID_FILE_NAMES:
+            with self.subTest(file_name=file_name):
+                self.assertIsNone(parser.sanitize_file_name(file_name))
+
     def test_rfc2231_parsing(self):
         test_data = (
             (b"Content-Type: application/x-stuff; title*=us-ascii'en-us'This%20is%20%2A%2A%2Afun%2A%2A%2A",
diff --git a/tests/forms_tests/field_tests/test_filefield.py b/tests/forms_tests/field_tests/test_filefield.py
index 59c91005d63e..a0a3e7b95364 100644
--- a/tests/forms_tests/field_tests/test_filefield.py
+++ b/tests/forms_tests/field_tests/test_filefield.py
@@ -23,10 +23,12 @@ class FileFieldTest(SimpleTestCase):
             f.clean(None, '')
         self.assertEqual('files/test2.pdf', f.clean(None, 'files/test2.pdf'))
         no_file_msg = "'No file was submitted. Check the encoding type on the form.'"
+        file = SimpleUploadedFile(None, b'')
+        file._name = ''
         with self.assertRaisesMessage(ValidationError, no_file_msg):
-            f.clean(SimpleUploadedFile('', b''))
+            f.clean(file)
         with self.assertRaisesMessage(ValidationError, no_file_msg):
-            f.clean(SimpleUploadedFile('', b''), '')
+            f.clean(file, '')
         self.assertEqual('files/test3.pdf', f.clean(None, 'files/test3.pdf'))
         with self.assertRaisesMessage(ValidationError, no_file_msg):
             f.clean('some content that is not a file')
diff --git a/tests/utils_tests/test_text.py b/tests/utils_tests/test_text.py
index bfc1b4efc44b..f26cbb97fcac 100644
--- a/tests/utils_tests/test_text.py
+++ b/tests/utils_tests/test_text.py
@@ -3,6 +3,7 @@ from __future__ import unicode_literals
 
 import json
 
+from django.core.exceptions import SuspiciousFileOperation
 from django.test import SimpleTestCase
 from django.utils import six, text
 from django.utils.functional import lazystr
@@ -233,6 +234,13 @@ class TestUtilsText(SimpleTestCase):
         filename = "^&'@{}[],$=!-#()%+~_123.txt"
         self.assertEqual(text.get_valid_filename(filename), "-_123.txt")
         self.assertEqual(text.get_valid_filename(lazystr(filename)), "-_123.txt")
+        msg = "Could not derive file name from '???'"
+        with self.assertRaisesMessage(SuspiciousFileOperation, msg):
+            text.get_valid_filename('???')
+        # After sanitizing this would yield '..'.
+        msg = "Could not derive file name from '$.$.$'"
+        with self.assertRaisesMessage(SuspiciousFileOperation, msg):
+            text.get_valid_filename('$.$.$')
 
     def test_compress_sequence(self):
         data = [{'key': i} for i in range(10)]

Places

File CVE-2021-31542.patch of Package python-Django

Places