[3.1.x] Fixed CVE-2021-31542 -- Tightened path & file name sanitation in file uploads.

6 files changed, 53 insertions, 7 deletions

diff --git a/django/core/files/storage.py b/django/core/files/storage.py
index 16f9d4e27b..3e68853b59 100644
--- a/django/core/files/storage.py
+++ b/django/core/files/storage.py
@@ -1,4 +1,5 @@
 import os
+import pathlib
 from datetime import datetime
 from urllib.parse import urljoin
 
@@ -6,6 +7,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 safe_join
@@ -74,6 +76,9 @@ class Storage:
         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, generate an alternative filename
         # until it doesn't exist.
@@ -105,6 +110,8 @@ class Storage:
         """
         # `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 48007b8682..f452bcd9a4 100644
--- a/django/core/files/uploadedfile.py
+++ b/django/core/files/uploadedfile.py
@@ -8,6 +8,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
 
 __all__ = ('UploadedFile', 'TemporaryUploadedFile', 'InMemoryUploadedFile',
            'SimpleUploadedFile')
@@ -47,6 +48,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 de89607175..f83cb1a3cf 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:
     """
     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 3a0bfacda2..b16fd05f5b 100644
--- a/django/db/models/fields/files.py
+++ b/django/db/models/fields/files.py
@@ -6,6 +6,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 Storage, 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.translation import gettext_lazy as _
@@ -318,6 +319,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 2351055e3a..38f21272bf 100644
--- a/django/http/multipartparser.py
+++ b/django/http/multipartparser.py
@@ -9,7 +9,6 @@ import binascii
 import cgi
 import collections
 import html
-import os
 from urllib.parse import unquote
 
 from django.conf import settings
@@ -299,10 +298,25 @@ class MultiPartParser:
                 break
 
     def sanitize_file_name(self, 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 = html.unescape(file_name)
-        # Cleanup Windows-style path separators.
-        file_name = file_name[file_name.rfind('\\') + 1:].strip()
-        return os.path.basename(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 fb5f6298c4..86594a0199 100644
--- a/django/utils/text.py
+++ b/django/utils/text.py
@@ -5,6 +5,7 @@ import warnings
 from gzip import GzipFile
 from io import BytesIO
 
+from django.core.exceptions import SuspiciousFileOperation
 from django.utils.deprecation import RemovedInDjango40Warning
 from django.utils.functional import SimpleLazyObject, keep_lazy_text, lazy
 from django.utils.regex_helper import _lazy_re_compile
@@ -219,7 +220,7 @@ class Truncator(SimpleLazyObject):
 
 
 @keep_lazy_text
-def get_valid_filename(s):
+def get_valid_filename(name):
     """
     Return the given string converted to a string that can be used for a clean
     filename. Remove leading and trailing spaces; convert other spaces to
@@ -228,8 +229,11 @@ def get_valid_filename(s):
     >>> get_valid_filename("john's portrait in 2004.jpg")
     'johns_portrait_in_2004.jpg'
     """
-    s = str(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