diff --git a/django/core/files/utils.py b/django/core/files/utils.py
index f83cb1a3cf..f28cea1077 100644
--- a/django/core/files/utils.py
+++ b/django/core/files/utils.py
@@ -1,16 +1,26 @@
import os
+import pathlib
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)
-
+def validate_file_name(name, allow_relative_path=False):
# Remove potentially dangerous names
- if name in {'', '.', '..'}:
+ if os.path.basename(name) in {'', '.', '..'}:
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)
+ 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
diff --git a/django/db/models/fields/files.py b/django/db/models/fields/files.py
index a2f972489f..18900f7b85 100644
--- a/django/db/models/fields/files.py
+++ b/django/db/models/fields/files.py
@@ -313,12 +313,12 @@ 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:
dirname = datetime.datetime.now().strftime(str(self.upload_to))
filename = posixpath.join(dirname, filename)
+ filename = validate_file_name(filename, allow_relative_path=True)
return self.storage.generate_filename(filename)
def save_form_data(self, instance, data):
diff --git a/docs/releases/2.2.23.txt b/docs/releases/2.2.23.txt
new file mode 100644
index 0000000000..6c39361e5f
--- /dev/null
+++ b/docs/releases/2.2.23.txt
@@ -0,0 +1,15 @@
+===========================
+Django 2.2.23 release notes
+===========================
+
+*May 13, 2021*
+
+Django 2.2.23 fixes a regression in 2.2.21.
+
+Bugfixes
+========
+
+* Fixed a regression in Django 2.2.21 where saving ``FileField`` would raise a
+ ``SuspiciousFileOperation`` even when a custom
+ :attr:`~django.db.models.FileField.upload_to` returns a valid file path
+ (:ticket:`32718`).
diff --git a/docs/releases/3.1.11.txt b/docs/releases/3.1.11.txt
new file mode 100644
index 0000000000..d5fb537466
--- /dev/null
+++ b/docs/releases/3.1.11.txt
@@ -0,0 +1,15 @@
+===========================
+Django 3.1.11 release notes
+===========================
+
+*May 13, 2021*
+
+Django 3.1.11 fixes a regression in 3.1.9.
+
+Bugfixes
+========
+
+* Fixed a regression in Django 3.1.9 where saving ``FileField`` would raise a
+ ``SuspiciousFileOperation`` even when a custom
+ :attr:`~django.db.models.FileField.upload_to` returns a valid file path
+ (:ticket:`32718`).
diff --git a/docs/releases/3.2.3.txt b/docs/releases/3.2.3.txt
index 315678b92a..915590f85c 100644
--- a/docs/releases/3.2.3.txt
+++ b/docs/releases/3.2.3.txt
@@ -2,7 +2,7 @@
Django 3.2.3 release notes
==========================
-*Expected June 1, 2021*
+*May 13, 2021*
Django 3.2.3 fixes several bugs in 3.2.2.
@@ -13,3 +13,8 @@ Bugfixes
* Fixed a regression in Django 3.2 that caused the incorrect filtering of
querysets combined with the ``|`` operator (:ticket:`32717`).
+
+* Fixed a regression in Django 3.2.1 where saving ``FileField`` would raise a
+ ``SuspiciousFileOperation`` even when a custom
+ :attr:`~django.db.models.FileField.upload_to` returns a valid file path
+ (:ticket:`32718`).
diff --git a/docs/releases/index.txt b/docs/releases/index.txt
index 529ad689f9..fdf46f272a 100644
--- a/docs/releases/index.txt
+++ b/docs/releases/index.txt
@@ -35,6 +35,7 @@ versions of the documentation contain the release notes for any later releases.
.. toctree::
:maxdepth: 1
+ 3.1.11
3.1.10
3.1.9
3.1.8
@@ -73,6 +74,7 @@ versions of the documentation contain the release notes for any later releases.
.. toctree::
:maxdepth: 1
+ 2.2.23
2.2.22
2.2.21
2.2.20
diff --git a/tests/file_storage/test_generate_filename.py b/tests/file_storage/test_generate_filename.py
index 4746a53f69..66551c495b 100644
--- a/tests/file_storage/test_generate_filename.py
+++ b/tests/file_storage/test_generate_filename.py
@@ -1,6 +1,4 @@
import os
-import sys
-from unittest import skipIf
from django.core.exceptions import SuspiciousFileOperation
from django.core.files.base import ContentFile
@@ -64,19 +62,37 @@ class GenerateFilenameStorageTests(SimpleTestCase):
s.generate_filename(file_name)
def test_filefield_dangerous_filename(self):
- candidates = ['..', '.', '', '???', '$.$.$']
+ candidates = [
+ ('..', 'some/folder/..'),
+ ('.', 'some/folder/.'),
+ ('', 'some/folder/'),
+ ('???', '???'),
+ ('$.$.$', '$.$.$'),
+ ]
f = FileField(upload_to='some/folder/')
- msg = "Could not derive file name from '%s'"
- for file_name in candidates:
+ for file_name, msg_file_name in candidates:
+ msg = f"Could not derive file name from '{msg_file_name}'"
with self.subTest(file_name=file_name):
- with self.assertRaisesMessage(SuspiciousFileOperation, msg % file_name):
+ with self.assertRaisesMessage(SuspiciousFileOperation, msg):
f.generate_filename(None, file_name)
- def test_filefield_dangerous_filename_dir(self):
+ def test_filefield_dangerous_filename_dot_segments(self):
f = FileField(upload_to='some/folder/')
- msg = "File name '/tmp/path' includes path elements"
+ msg = "Detected path traversal attempt in 'some/folder/../path'"
with self.assertRaisesMessage(SuspiciousFileOperation, msg):
- f.generate_filename(None, '/tmp/path')
+ f.generate_filename(None, '../path')
+
+ def test_filefield_generate_filename_absolute_path(self):
+ f = FileField(upload_to='some/folder/')
+ candidates = [
+ '/tmp/path',
+ '/tmp/../path',
+ ]
+ for file_name in candidates:
+ msg = f"Detected path traversal attempt in '{file_name}'"
+ with self.subTest(file_name=file_name):
+ with self.assertRaisesMessage(SuspiciousFileOperation, msg):
+ f.generate_filename(None, file_name)
def test_filefield_generate_filename(self):
f = FileField(upload_to='some/folder/')
@@ -95,7 +111,57 @@ class GenerateFilenameStorageTests(SimpleTestCase):
os.path.normpath('some/folder/test_with_space.txt')
)
- @skipIf(sys.platform == 'win32', 'Path components in filename are not supported after 0b79eb3.')
+ def test_filefield_generate_filename_upload_to_overrides_dangerous_filename(self):
+ def upload_to(instance, filename):
+ return 'test.txt'
+
+ f = FileField(upload_to=upload_to)
+ candidates = [
+ '/tmp/.',
+ '/tmp/..',
+ '/tmp/../path',
+ '/tmp/path',
+ 'some/folder/',
+ 'some/folder/.',
+ 'some/folder/..',
+ 'some/folder/???',
+ 'some/folder/$.$.$',
+ 'some/../test.txt',
+ '',
+ ]
+ for file_name in candidates:
+ with self.subTest(file_name=file_name):
+ self.assertEqual(f.generate_filename(None, file_name), 'test.txt')
+
+ def test_filefield_generate_filename_upload_to_absolute_path(self):
+ def upload_to(instance, filename):
+ return '/tmp/' + filename
+
+ f = FileField(upload_to=upload_to)
+ candidates = [
+ 'path',
+ '../path',
+ '???',
+ '$.$.$',
+ ]
+ for file_name in candidates:
+ msg = f"Detected path traversal attempt in '/tmp/{file_name}'"
+ with self.subTest(file_name=file_name):
+ with self.assertRaisesMessage(SuspiciousFileOperation, msg):
+ f.generate_filename(None, file_name)
+
+ def test_filefield_generate_filename_upload_to_dangerous_filename(self):
+ def upload_to(instance, filename):
+ return '/tmp/' + filename
+
+ f = FileField(upload_to=upload_to)
+ candidates = ['..', '.', '']
+ for file_name in candidates:
+ msg = f"Could not derive file name from '/tmp/{file_name}'"
+ with self.subTest(file_name=file_name):
+ with self.assertRaisesMessage(SuspiciousFileOperation, msg):
+ f.generate_filename(None, file_name)
+
def test_filefield_awss3_storage(self):
"""
Simulate a FileField with an S3 storage which uses keys rather than
diff --git a/tests/model_fields/test_filefield.py b/tests/model_fields/test_filefield.py
index 51e29f6d25..fb9426e2d1 100644
--- a/tests/model_fields/test_filefield.py
+++ b/tests/model_fields/test_filefield.py
@@ -5,6 +5,7 @@ import tempfile
import unittest
from pathlib import Path
+from django.core.exceptions import SuspiciousFileOperation
from django.core.files import File, temp
from django.core.files.base import ContentFile
from django.core.files.uploadedfile import TemporaryUploadedFile
@@ -63,6 +64,15 @@ class FileFieldTests(TestCase):
d.refresh_from_db()
self.assertIs(d.myfile.instance, d)
+ @unittest.skipIf(sys.platform == 'win32', "Crashes with OSError on Windows.")
+ def test_save_without_name(self):
+ with tempfile.NamedTemporaryFile(suffix='.txt') as tmp:
+ document = Document.objects.create(myfile='something.txt')
+ document.myfile = File(tmp)
+ msg = f"Detected path traversal attempt in '{tmp.name}'"
+ with self.assertRaisesMessage(SuspiciousFileOperation, msg):
+ document.save()
+
def test_defer(self):
Document.objects.create(myfile='something.txt')
self.assertEqual(Document.objects.defer('myfile')[0].myfile, 'something.txt')