mirrors/django
1
0
Fork 0
mirror of https://github.com/django/django.git synced 2024-12-22 09:05:43 +00:00
Code Issues 25 Releases Wiki Activity

Fixed CVE-2023-31047, Fixed #31710 -- Prevented potential bypass of validation when uploading multiple files using one form field.

Browse Source 
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>
This commit is contained in:
Mariusz Felisiak 2023-04-13 10:10:56 +02:00
parent 8e2460d599
commit fb4c55d9ec
8 changed files with 245 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
django/forms/widgets.py
View File

@ -406,17 +406,41 @@ class MultipleHiddenInput(HiddenInput):
class FileInput(Input):
allow_multiple_selected = False
input_type = "file"
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

16
docs/releases/3.2.19.txt
View File

@ -6,4 +6,18 @@ Django 3.2.19 release notes
Django 3.2.19 fixes a security issue with severity "low" in 3.2.18.
...
CVE-2023-31047: Potential bypass of validation when uploading multiple files using one form field
=================================================================================================
Uploading multiple files using one form field has never been supported by
:class:`.forms.FileField` or :class:`.forms.ImageField` as only the last
uploaded file was validated. Unfortunately, :ref:`uploading_multiple_files`
topic suggested otherwise.
In order to avoid the vulnerability, :class:`~django.forms.ClearableFileInput`
and :class:`~django.forms.FileInput` form widgets now raise ``ValueError`` when
the ``multiple`` HTML attribute is set on them. To prevent the exception and
keep the old behavior, set ``allow_multiple_selected`` to ``True``.
For more details on using the new attribute and handling of multiple files
through a single field, see :ref:`uploading_multiple_files`.

16
docs/releases/4.1.9.txt
View File

@ -6,4 +6,18 @@ Django 4.1.9 release notes
Django 4.1.9 fixes a security issue with severity "low" in 4.1.8.
...
CVE-2023-31047: Potential bypass of validation when uploading multiple files using one form field
=================================================================================================
Uploading multiple files using one form field has never been supported by
:class:`.forms.FileField` or :class:`.forms.ImageField` as only the last
uploaded file was validated. Unfortunately, :ref:`uploading_multiple_files`
topic suggested otherwise.
In order to avoid the vulnerability, :class:`~django.forms.ClearableFileInput`
and :class:`~django.forms.FileInput` form widgets now raise ``ValueError`` when
the ``multiple`` HTML attribute is set on them. To prevent the exception and
keep the old behavior, set ``allow_multiple_selected`` to ``True``.
For more details on using the new attribute and handling of multiple files
through a single field, see :ref:`uploading_multiple_files`.

16
docs/releases/4.2.1.txt
View File

@ -7,6 +7,22 @@ Django 4.2.1 release notes
Django 4.2.1 fixes a security issue with severity "low" and several bugs in
4.2.
CVE-2023-31047: Potential bypass of validation when uploading multiple files using one form field
=================================================================================================
Uploading multiple files using one form field has never been supported by
:class:`.forms.FileField` or :class:`.forms.ImageField` as only the last
uploaded file was validated. Unfortunately, :ref:`uploading_multiple_files`
topic suggested otherwise.
In order to avoid the vulnerability, :class:`~django.forms.ClearableFileInput`
and :class:`~django.forms.FileInput` form widgets now raise ``ValueError`` when
the ``multiple`` HTML attribute is set on them. To prevent the exception and
keep the old behavior, set ``allow_multiple_selected`` to ``True``.
For more details on using the new attribute and handling of multiple files
through a single field, see :ref:`uploading_multiple_files`.
Bugfixes
========

66
docs/topics/http/file-uploads.txt
View File

@ -144,11 +144,27 @@ a :class:`~django.core.files.File` like object to the
instance = ModelWithFileField(file_field=content_file)
instance.save()
.. _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``
@ -156,10 +172,26 @@ HTML attribute of field's widget:
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
@ -180,14 +212,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
===============

68
tests/forms_tests/field_tests/test_filefield.py
View File

@ -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
@ -109,3 +110,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)

5
tests/forms_tests/widget_tests/test_clearablefileinput.py
View File

@ -245,3 +245,8 @@ class ClearableFileInputTest(WidgetTest):
'<input type="file" name="clearable_file" id="id_clearable_file"></div>',
form.render(),
)
def test_multiple_error(self):
msg = "ClearableFileInput doesn't support uploading multiple files."
with self.assertRaisesMessage(ValueError, msg):
ClearableFileInput(attrs={"multiple": True})

44
tests/forms_tests/widget_tests/test_fileinput.py
View File

@ -1,4 +1,6 @@
from django.core.files.uploadedfile import SimpleUploadedFile
from django.forms import FileField, FileInput, Form
from django.utils.datastructures import MultiValueDict
from .base import WidgetTest
@ -48,3 +50,45 @@ class FileInputTest(WidgetTest):
'name="field" required type="file"></div>',
form.render(),
)
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)