mirror of
https://github.com/django/django.git
synced 2025-03-20 14:20:44 +00:00
[4.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>
This commit is contained in:
parent
290fd5ecec
commit
21b1b1fc03
@ -413,17 +413,41 @@ class MultipleHiddenInput(HiddenInput):
|
|||||||
|
|
||||||
|
|
||||||
class FileInput(Input):
|
class FileInput(Input):
|
||||||
|
allow_multiple_selected = False
|
||||||
input_type = "file"
|
input_type = "file"
|
||||||
needs_multipart_form = True
|
needs_multipart_form = True
|
||||||
template_name = "django/forms/widgets/file.html"
|
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):
|
def format_value(self, value):
|
||||||
"""File input never renders a value."""
|
"""File input never renders a value."""
|
||||||
return
|
return
|
||||||
|
|
||||||
def value_from_datadict(self, data, files, name):
|
def value_from_datadict(self, data, files, name):
|
||||||
"File widgets take data from FILES, not POST"
|
"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):
|
def value_omitted_from_data(self, data, files, name):
|
||||||
return name not in files
|
return name not in files
|
||||||
|
@ -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.
|
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`.
|
||||||
|
@ -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.
|
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`.
|
||||||
|
@ -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
|
Django 4.2.1 fixes a security issue with severity "low" and several bugs in
|
||||||
4.2.
|
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
|
Bugfixes
|
||||||
========
|
========
|
||||||
|
|
||||||
|
@ -144,11 +144,27 @@ a :class:`~django.core.files.File` like object to the
|
|||||||
instance = ModelWithFileField(file_field=content_file)
|
instance = ModelWithFileField(file_field=content_file)
|
||||||
instance.save()
|
instance.save()
|
||||||
|
|
||||||
|
.. _uploading_multiple_files:
|
||||||
|
|
||||||
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
|
.. code-block:: python
|
||||||
:caption: ``forms.py``
|
:caption: ``forms.py``
|
||||||
@ -156,10 +172,26 @@ HTML attribute of field's widget:
|
|||||||
from django import forms
|
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):
|
class FileFieldForm(forms.Form):
|
||||||
file_field = forms.FileField(
|
file_field = MultipleFileField()
|
||||||
widget=forms.ClearableFileInput(attrs={"multiple": True})
|
|
||||||
)
|
|
||||||
|
|
||||||
Then override the ``post`` method of your
|
Then override the ``post`` method of your
|
||||||
:class:`~django.views.generic.edit.FormView` subclass to handle multiple file
|
:class:`~django.views.generic.edit.FormView` subclass to handle multiple file
|
||||||
@ -180,14 +212,32 @@ uploads:
|
|||||||
def post(self, request, *args, **kwargs):
|
def post(self, request, *args, **kwargs):
|
||||||
form_class = self.get_form_class()
|
form_class = self.get_form_class()
|
||||||
form = self.get_form(form_class)
|
form = self.get_form(form_class)
|
||||||
files = request.FILES.getlist("file_field")
|
|
||||||
if form.is_valid():
|
if form.is_valid():
|
||||||
for f in files:
|
|
||||||
... # Do something with each file.
|
|
||||||
return self.form_valid(form)
|
return self.form_valid(form)
|
||||||
else:
|
else:
|
||||||
return self.form_invalid(form)
|
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
|
Upload Handlers
|
||||||
===============
|
===============
|
||||||
|
|
||||||
|
@ -2,7 +2,8 @@ import pickle
|
|||||||
|
|
||||||
from django.core.exceptions import ValidationError
|
from django.core.exceptions import ValidationError
|
||||||
from django.core.files.uploadedfile import SimpleUploadedFile
|
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
|
from django.test import SimpleTestCase
|
||||||
|
|
||||||
|
|
||||||
@ -109,3 +110,68 @@ class FileFieldTest(SimpleTestCase):
|
|||||||
|
|
||||||
def test_file_picklable(self):
|
def test_file_picklable(self):
|
||||||
self.assertIsInstance(pickle.loads(pickle.dumps(FileField())), FileField)
|
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)
|
||||||
|
@ -242,3 +242,8 @@ class ClearableFileInputTest(WidgetTest):
|
|||||||
'<input type="file" name="clearable_file" id="id_clearable_file"></div>',
|
'<input type="file" name="clearable_file" id="id_clearable_file"></div>',
|
||||||
form.render(),
|
form.render(),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def test_multiple_error(self):
|
||||||
|
msg = "ClearableFileInput doesn't support uploading multiple files."
|
||||||
|
with self.assertRaisesMessage(ValueError, msg):
|
||||||
|
ClearableFileInput(attrs={"multiple": True})
|
||||||
|
@ -1,4 +1,6 @@
|
|||||||
|
from django.core.files.uploadedfile import SimpleUploadedFile
|
||||||
from django.forms import FileField, FileInput, Form
|
from django.forms import FileField, FileInput, Form
|
||||||
|
from django.utils.datastructures import MultiValueDict
|
||||||
|
|
||||||
from .base import WidgetTest
|
from .base import WidgetTest
|
||||||
|
|
||||||
@ -48,3 +50,45 @@ class FileInputTest(WidgetTest):
|
|||||||
'name="field" required type="file"></div>',
|
'name="field" required type="file"></div>',
|
||||||
form.render(),
|
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)
|
||||||
|
Loading…
x
Reference in New Issue
Block a user