323 lines
12 KiB
Diff
323 lines
12 KiB
Diff
From 6bb2e1ac607b1a399e1d7bd3650c04a586e6746e Mon Sep 17 00:00:00 2001
|
|
From: starlet-dx <15929766099@163.com>
|
|
Date: Tue, 16 May 2023 10:00:42 +0800
|
|
Subject: [PATCH 1/1] [3.2.x] Fixed CVE-2023-31047, Fixed #31710 -- Prevented
|
|
potential bypass of validation when uploading multiple files using one form
|
|
field.
|
|
|
|
Thanks Moataz Al-Sharida and nawaik for reports.
|
|
|
|
Co-authored-by: Shai Berger <shai@platonix.com>
|
|
Co-authored-by: nessita <124304+nessita@users.noreply.github.com>
|
|
|
|
Origin:
|
|
https://github.com/django/django/commit/eed53d0011622e70b936e203005f0e6f4ac48965
|
|
---
|
|
django/forms/widgets.py | 26 ++++++-
|
|
docs/topics/http/file-uploads.txt | 65 ++++++++++++++++--
|
|
.../forms_tests/field_tests/test_filefield.py | 68 ++++++++++++++++++-
|
|
.../widget_tests/test_clearablefileinput.py | 5 ++
|
|
.../widget_tests/test_fileinput.py | 44 ++++++++++++
|
|
5 files changed, 200 insertions(+), 8 deletions(-)
|
|
|
|
diff --git a/django/forms/widgets.py b/django/forms/widgets.py
|
|
index 1b1c143..8ef8255 100644
|
|
--- a/django/forms/widgets.py
|
|
+++ b/django/forms/widgets.py
|
|
@@ -378,16 +378,40 @@ class MultipleHiddenInput(HiddenInput):
|
|
|
|
class FileInput(Input):
|
|
input_type = 'file'
|
|
+ allow_multiple_selected = False
|
|
needs_multipart_form = True
|
|
template_name = 'django/forms/widgets/file.html'
|
|
|
|
+ def __init__(self, attrs=None):
|
|
+ if (
|
|
+ attrs is not None and
|
|
+ not self.allow_multiple_selected and
|
|
+ attrs.get("multiple", False)
|
|
+ ):
|
|
+ raise ValueError(
|
|
+ "%s doesn't support uploading multiple files."
|
|
+ % self.__class__.__qualname__
|
|
+ )
|
|
+ if self.allow_multiple_selected:
|
|
+ if attrs is None:
|
|
+ attrs = {"multiple": True}
|
|
+ else:
|
|
+ attrs.setdefault("multiple", True)
|
|
+ super().__init__(attrs)
|
|
+
|
|
def format_value(self, value):
|
|
"""File input never renders a value."""
|
|
return
|
|
|
|
def value_from_datadict(self, data, files, name):
|
|
"File widgets take data from FILES, not POST"
|
|
- return files.get(name)
|
|
+ getter = files.get
|
|
+ if self.allow_multiple_selected:
|
|
+ try:
|
|
+ getter = files.getlist
|
|
+ except AttributeError:
|
|
+ pass
|
|
+ return getter(name)
|
|
|
|
def value_omitted_from_data(self, data, files, name):
|
|
return name not in files
|
|
diff --git a/docs/topics/http/file-uploads.txt b/docs/topics/http/file-uploads.txt
|
|
index ca272d7..4388594 100644
|
|
--- a/docs/topics/http/file-uploads.txt
|
|
+++ b/docs/topics/http/file-uploads.txt
|
|
@@ -126,19 +126,54 @@ model::
|
|
form = UploadFileForm()
|
|
return render(request, 'upload.html', {'form': form})
|
|
|
|
+.. _uploading_multiple_files:
|
|
+
|
|
Uploading multiple files
|
|
------------------------
|
|
|
|
-If you want to upload multiple files using one form field, set the ``multiple``
|
|
-HTML attribute of field's widget:
|
|
+..
|
|
+ Tests in tests.forms_tests.field_tests.test_filefield.MultipleFileFieldTest
|
|
+ should be updated after any changes in the following snippets.
|
|
+
|
|
+If you want to upload multiple files using one form field, create a subclass
|
|
+of the field's widget and set the ``allow_multiple_selected`` attribute on it
|
|
+to ``True``.
|
|
+
|
|
+In order for such files to be all validated by your form (and have the value of
|
|
+the field include them all), you will also have to subclass ``FileField``. See
|
|
+below for an example.
|
|
+
|
|
+.. admonition:: Multiple file field
|
|
+
|
|
+ Django is likely to have a proper multiple file field support at some point
|
|
+ in the future.
|
|
|
|
.. code-block:: python
|
|
:caption: forms.py
|
|
|
|
from django import forms
|
|
|
|
+
|
|
+ class MultipleFileInput(forms.ClearableFileInput):
|
|
+ allow_multiple_selected = True
|
|
+
|
|
+
|
|
+ class MultipleFileField(forms.FileField):
|
|
+ def __init__(self, *args, **kwargs):
|
|
+ kwargs.setdefault("widget", MultipleFileInput())
|
|
+ super().__init__(*args, **kwargs)
|
|
+
|
|
+ def clean(self, data, initial=None):
|
|
+ single_file_clean = super().clean
|
|
+ if isinstance(data, (list, tuple)):
|
|
+ result = [single_file_clean(d, initial) for d in data]
|
|
+ else:
|
|
+ result = single_file_clean(data, initial)
|
|
+ return result
|
|
+
|
|
+
|
|
class FileFieldForm(forms.Form):
|
|
- file_field = forms.FileField(widget=forms.ClearableFileInput(attrs={'multiple': True}))
|
|
+ file_field = MultipleFileField()
|
|
|
|
Then override the ``post`` method of your
|
|
:class:`~django.views.generic.edit.FormView` subclass to handle multiple file
|
|
@@ -158,14 +193,32 @@ uploads:
|
|
def post(self, request, *args, **kwargs):
|
|
form_class = self.get_form_class()
|
|
form = self.get_form(form_class)
|
|
- files = request.FILES.getlist('file_field')
|
|
if form.is_valid():
|
|
- for f in files:
|
|
- ... # Do something with each file.
|
|
return self.form_valid(form)
|
|
else:
|
|
return self.form_invalid(form)
|
|
|
|
+ def form_valid(self, form):
|
|
+ files = form.cleaned_data["file_field"]
|
|
+ for f in files:
|
|
+ ... # Do something with each file.
|
|
+ return super().form_valid()
|
|
+
|
|
+.. warning::
|
|
+
|
|
+ This will allow you to handle multiple files at the form level only. Be
|
|
+ aware that you cannot use it to put multiple files on a single model
|
|
+ instance (in a single field), for example, even if the custom widget is used
|
|
+ with a form field related to a model ``FileField``.
|
|
+
|
|
+.. versionchanged:: 3.2.19
|
|
+
|
|
+ In previous versions, there was no support for the ``allow_multiple_selected``
|
|
+ class attribute, and users were advised to create the widget with the HTML
|
|
+ attribute ``multiple`` set through the ``attrs`` argument. However, this
|
|
+ caused validation of the form field to be applied only to the last file
|
|
+ submitted, which could have adverse security implications.
|
|
+
|
|
Upload Handlers
|
|
===============
|
|
|
|
diff --git a/tests/forms_tests/field_tests/test_filefield.py b/tests/forms_tests/field_tests/test_filefield.py
|
|
index 2db106e..b54febd 100644
|
|
--- a/tests/forms_tests/field_tests/test_filefield.py
|
|
+++ b/tests/forms_tests/field_tests/test_filefield.py
|
|
@@ -2,7 +2,8 @@ import pickle
|
|
|
|
from django.core.exceptions import ValidationError
|
|
from django.core.files.uploadedfile import SimpleUploadedFile
|
|
-from django.forms import FileField
|
|
+from django.core.validators import validate_image_file_extension
|
|
+from django.forms import FileField, FileInput
|
|
from django.test import SimpleTestCase
|
|
|
|
|
|
@@ -83,3 +84,68 @@ class FileFieldTest(SimpleTestCase):
|
|
|
|
def test_file_picklable(self):
|
|
self.assertIsInstance(pickle.loads(pickle.dumps(FileField())), FileField)
|
|
+
|
|
+
|
|
+class MultipleFileInput(FileInput):
|
|
+ allow_multiple_selected = True
|
|
+
|
|
+
|
|
+class MultipleFileField(FileField):
|
|
+ def __init__(self, *args, **kwargs):
|
|
+ kwargs.setdefault("widget", MultipleFileInput())
|
|
+ super().__init__(*args, **kwargs)
|
|
+
|
|
+ def clean(self, data, initial=None):
|
|
+ single_file_clean = super().clean
|
|
+ if isinstance(data, (list, tuple)):
|
|
+ result = [single_file_clean(d, initial) for d in data]
|
|
+ else:
|
|
+ result = single_file_clean(data, initial)
|
|
+ return result
|
|
+
|
|
+
|
|
+class MultipleFileFieldTest(SimpleTestCase):
|
|
+ def test_file_multiple(self):
|
|
+ f = MultipleFileField()
|
|
+ files = [
|
|
+ SimpleUploadedFile("name1", b"Content 1"),
|
|
+ SimpleUploadedFile("name2", b"Content 2"),
|
|
+ ]
|
|
+ self.assertEqual(f.clean(files), files)
|
|
+
|
|
+ def test_file_multiple_empty(self):
|
|
+ f = MultipleFileField()
|
|
+ files = [
|
|
+ SimpleUploadedFile("empty", b""),
|
|
+ SimpleUploadedFile("nonempty", b"Some Content"),
|
|
+ ]
|
|
+ msg = "'The submitted file is empty.'"
|
|
+ with self.assertRaisesMessage(ValidationError, msg):
|
|
+ f.clean(files)
|
|
+ with self.assertRaisesMessage(ValidationError, msg):
|
|
+ f.clean(files[::-1])
|
|
+
|
|
+ def test_file_multiple_validation(self):
|
|
+ f = MultipleFileField(validators=[validate_image_file_extension])
|
|
+
|
|
+ good_files = [
|
|
+ SimpleUploadedFile("image1.jpg", b"fake JPEG"),
|
|
+ SimpleUploadedFile("image2.png", b"faux image"),
|
|
+ SimpleUploadedFile("image3.bmp", b"fraudulent bitmap"),
|
|
+ ]
|
|
+ self.assertEqual(f.clean(good_files), good_files)
|
|
+
|
|
+ evil_files = [
|
|
+ SimpleUploadedFile("image1.sh", b"#!/bin/bash -c 'echo pwned!'\n"),
|
|
+ SimpleUploadedFile("image2.png", b"faux image"),
|
|
+ SimpleUploadedFile("image3.jpg", b"fake JPEG"),
|
|
+ ]
|
|
+
|
|
+ evil_rotations = (
|
|
+ evil_files[i:] + evil_files[:i] # Rotate by i.
|
|
+ for i in range(len(evil_files))
|
|
+ )
|
|
+ msg = "File extension “sh” is not allowed. Allowed extensions are: "
|
|
+ for rotated_evil_files in evil_rotations:
|
|
+ with self.assertRaisesMessage(ValidationError, msg):
|
|
+ f.clean(rotated_evil_files)
|
|
diff --git a/tests/forms_tests/widget_tests/test_clearablefileinput.py b/tests/forms_tests/widget_tests/test_clearablefileinput.py
|
|
index dee44c4..6cf1476 100644
|
|
--- a/tests/forms_tests/widget_tests/test_clearablefileinput.py
|
|
+++ b/tests/forms_tests/widget_tests/test_clearablefileinput.py
|
|
@@ -176,3 +176,8 @@ class ClearableFileInputTest(WidgetTest):
|
|
self.assertIs(widget.value_omitted_from_data({}, {}, 'field'), True)
|
|
self.assertIs(widget.value_omitted_from_data({}, {'field': 'x'}, 'field'), False)
|
|
self.assertIs(widget.value_omitted_from_data({'field-clear': 'y'}, {}, 'field'), False)
|
|
+
|
|
+ def test_multiple_error(self):
|
|
+ msg = "ClearableFileInput doesn't support uploading multiple files."
|
|
+ with self.assertRaisesMessage(ValueError, msg):
|
|
+ ClearableFileInput(attrs={"multiple": True})
|
|
diff --git a/tests/forms_tests/widget_tests/test_fileinput.py b/tests/forms_tests/widget_tests/test_fileinput.py
|
|
index 8eec262..8068f70 100644
|
|
--- a/tests/forms_tests/widget_tests/test_fileinput.py
|
|
+++ b/tests/forms_tests/widget_tests/test_fileinput.py
|
|
@@ -1,4 +1,6 @@
|
|
+from django.core.files.uploadedfile import SimpleUploadedFile
|
|
from django.forms import FileInput
|
|
+from django.utils.datastructures import MultiValueDict
|
|
|
|
from .base import WidgetTest
|
|
|
|
@@ -24,3 +26,45 @@ class FileInputTest(WidgetTest):
|
|
# user to keep the existing, initial value.
|
|
self.assertIs(self.widget.use_required_attribute(None), True)
|
|
self.assertIs(self.widget.use_required_attribute('resume.txt'), False)
|
|
+
|
|
+ def test_multiple_error(self):
|
|
+ msg = "FileInput doesn't support uploading multiple files."
|
|
+ with self.assertRaisesMessage(ValueError, msg):
|
|
+ FileInput(attrs={"multiple": True})
|
|
+
|
|
+ def test_value_from_datadict_multiple(self):
|
|
+ class MultipleFileInput(FileInput):
|
|
+ allow_multiple_selected = True
|
|
+
|
|
+ file_1 = SimpleUploadedFile("something1.txt", b"content 1")
|
|
+ file_2 = SimpleUploadedFile("something2.txt", b"content 2")
|
|
+ # Uploading multiple files is allowed.
|
|
+ widget = MultipleFileInput(attrs={"multiple": True})
|
|
+ value = widget.value_from_datadict(
|
|
+ data={"name": "Test name"},
|
|
+ files=MultiValueDict({"myfile": [file_1, file_2]}),
|
|
+ name="myfile",
|
|
+ )
|
|
+ self.assertEqual(value, [file_1, file_2])
|
|
+ # Uploading multiple files is not allowed.
|
|
+ widget = FileInput()
|
|
+ value = widget.value_from_datadict(
|
|
+ data={"name": "Test name"},
|
|
+ files=MultiValueDict({"myfile": [file_1, file_2]}),
|
|
+ name="myfile",
|
|
+ )
|
|
+ self.assertEqual(value, file_2)
|
|
+
|
|
+ def test_multiple_default(self):
|
|
+ class MultipleFileInput(FileInput):
|
|
+ allow_multiple_selected = True
|
|
+
|
|
+ tests = [
|
|
+ (None, True),
|
|
+ ({"class": "myclass"}, True),
|
|
+ ({"multiple": False}, False),
|
|
+ ]
|
|
+ for attrs, expected in tests:
|
|
+ with self.subTest(attrs=attrs):
|
|
+ widget = MultipleFileInput(attrs=attrs)
|
|
+ self.assertIs(widget.attrs["multiple"], expected)
|
|
--
|
|
2.30.0
|
|
|