summaryrefslogtreecommitdiff
path: root/tests/file_storage
diff options
context:
space:
mode:
authorMariusz Felisiak <felisiak.mariusz@gmail.com>2021-05-13 08:53:44 +0200
committerGitHub <noreply@github.com>2021-05-13 08:53:44 +0200
commitb55699968fc9ee985384c64e37f6cc74a0a23683 (patch)
treeec84729969a2c35777792ef5ca2df54d238391ff /tests/file_storage
parentb81c7562fc33f50166d5120138d6398dc42b13c3 (diff)
downloaddjango-b55699968fc9ee985384c64e37f6cc74a0a23683.tar.gz
Fixed #32718 -- Relaxed file name validation in FileField.
- Validate filename returned by FileField.upload_to() not a filename passed to the FileField.generate_filename() (upload_to() may completely ignored passed filename). - Allow relative paths (without dot segments) in the generated filename. Thanks to Jakub Kleň for the report and review. Thanks to all folks for checking this patch on existing projects. Thanks Florian Apolloner and Markus Holtermann for the discussion and implementation idea. Regression in 0b79eb36915d178aef5c6a7bbce71b1e76d376d3.
Diffstat (limited to 'tests/file_storage')
-rw-r--r--tests/file_storage/test_generate_filename.py86
1 files changed, 76 insertions, 10 deletions
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