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-24580 -- Prevented DoS with too many uploaded files.

Browse Source 
Thanks to Jakob Ackermann for the report.
This commit is contained in:
Markus Holtermann 2022-12-13 10:27:39 +01:00 committed by Carlton Gibson
parent 1eb94bc8da
commit 85ac33591c
12 changed files with 213 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
django/conf/global_settings.py
View File

@ -313,6 +313,10 @@ DATA_UPLOAD_MAX_MEMORY_SIZE = 2621440 # i.e. 2.5 MB
# SuspiciousOperation (TooManyFieldsSent) is raised.
DATA_UPLOAD_MAX_NUMBER_FIELDS = 1000
# Maximum number of files encoded in a multipart upload that will be read
# before a SuspiciousOperation (TooManyFilesSent) is raised.
DATA_UPLOAD_MAX_NUMBER_FILES = 100
# Directory in which upload streamed files will be temporarily saved. A value of
# `None` will make Django use the operating system's default temporary directory
# (i.e. "/tmp" on *nix systems).

9
django/core/exceptions.py
View File

@ -67,6 +67,15 @@ class TooManyFieldsSent(SuspiciousOperation):
pass
class TooManyFilesSent(SuspiciousOperation):
"""
The number of fields in a GET or POST request exceeded
settings.DATA_UPLOAD_MAX_NUMBER_FILES.
"""
pass
class RequestDataTooBig(SuspiciousOperation):
"""
The size of the request (excluding any file uploads) exceeded

3
django/core/handlers/exception.py
View File

@ -12,6 +12,7 @@ from django.core.exceptions import (
RequestDataTooBig,
SuspiciousOperation,
TooManyFieldsSent,
TooManyFilesSent,
)
from django.http import Http404
from django.http.multipartparser import MultiPartParserError
@ -110,7 +111,7 @@ def response_for_exception(request, exc):
exception=exc,
)
elif isinstance(exc, SuspiciousOperation):
if isinstance(exc, (RequestDataTooBig, TooManyFieldsSent)):
if isinstance(exc, (RequestDataTooBig, TooManyFieldsSent, TooManyFilesSent)):
# POST data can't be accessed again, otherwise the original
# exception would be raised.
request._mark_post_parse_error()

64
django/http/multipartparser.py
View File

@ -14,6 +14,7 @@ from django.core.exceptions import (
RequestDataTooBig,
SuspiciousMultipartForm,
TooManyFieldsSent,
TooManyFilesSent,
)
from django.core.files.uploadhandler import SkipFile, StopFutureHandlers, StopUpload
from django.utils.datastructures import MultiValueDict
@ -39,6 +40,7 @@ class InputStreamExhausted(Exception):
RAW = "raw"
FILE = "file"
FIELD = "field"
FIELD_TYPES = frozenset([FIELD, RAW])
class MultiPartParser:
@ -111,6 +113,22 @@ class MultiPartParser:
self._upload_handlers = upload_handlers
def parse(self):
# Call the actual parse routine and close all open files in case of
# errors. This is needed because if exceptions are thrown the
# MultiPartParser will not be garbage collected immediately and
# resources would be kept alive. This is only needed for errors because
# the Request object closes all uploaded files at the end of the
# request.
try:
return self._parse()
except Exception:
if hasattr(self, "_files"):
for _, files in self._files.lists():
for fileobj in files:
fileobj.close()
raise
def _parse(self):
"""
Parse the POST data and break it into a FILES MultiValueDict and a POST
MultiValueDict.
@ -156,6 +174,8 @@ class MultiPartParser:
num_bytes_read = 0
# To count the number of keys in the request.
num_post_keys = 0
# To count the number of files in the request.
num_files = 0
# To limit the amount of data read from the request.
read_size = None
# Whether a file upload is finished.
@ -171,6 +191,20 @@ class MultiPartParser:
old_field_name = None
uploaded_file = True
if (
item_type in FIELD_TYPES
and settings.DATA_UPLOAD_MAX_NUMBER_FIELDS is not None
):
# Avoid storing more than DATA_UPLOAD_MAX_NUMBER_FIELDS.
num_post_keys += 1
# 2 accounts for empty raw fields before and after the
# last boundary.
if settings.DATA_UPLOAD_MAX_NUMBER_FIELDS + 2 < num_post_keys:
raise TooManyFieldsSent(
"The number of GET/POST parameters exceeded "
"settings.DATA_UPLOAD_MAX_NUMBER_FIELDS."
)
try:
disposition = meta_data["content-disposition"][1]
field_name = disposition["name"].strip()
@ -183,17 +217,6 @@ class MultiPartParser:
field_name = force_str(field_name, encoding, errors="replace")
if item_type == FIELD:
# Avoid storing more than DATA_UPLOAD_MAX_NUMBER_FIELDS.
num_post_keys += 1
if (
settings.DATA_UPLOAD_MAX_NUMBER_FIELDS is not None
and settings.DATA_UPLOAD_MAX_NUMBER_FIELDS < num_post_keys
):
raise TooManyFieldsSent(
"The number of GET/POST parameters exceeded "
"settings.DATA_UPLOAD_MAX_NUMBER_FIELDS."
)
# Avoid reading more than DATA_UPLOAD_MAX_MEMORY_SIZE.
if settings.DATA_UPLOAD_MAX_MEMORY_SIZE is not None:
read_size = (
@ -228,6 +251,16 @@ class MultiPartParser:
field_name, force_str(data, encoding, errors="replace")
)
elif item_type == FILE:
# Avoid storing more than DATA_UPLOAD_MAX_NUMBER_FILES.
num_files += 1
if (
settings.DATA_UPLOAD_MAX_NUMBER_FILES is not None
and num_files > settings.DATA_UPLOAD_MAX_NUMBER_FILES
):
raise TooManyFilesSent(
"The number of files exceeded "
"settings.DATA_UPLOAD_MAX_NUMBER_FILES."
)
# This is a file, use the handler...
file_name = disposition.get("filename")
if file_name:
@ -305,8 +338,13 @@ class MultiPartParser:
# Handle file upload completions on next iteration.
old_field_name = field_name
else:
# If this is neither a FIELD or a FILE, just exhaust the stream.
exhaust(stream)
# If this is neither a FIELD nor a FILE, exhaust the field
# stream. Note: There could be an error here at some point,
# but there will be at least two RAW types (before and
# after the other boundaries). This branch is usually not
# reached at all, because a missing content-disposition
# header will skip the whole boundary.
exhaust(field_stream)
except StopUpload as e:
self._close_files()
if not e.connection_reset:

8
django/http/request.py
View File

@ -13,7 +13,11 @@ from django.core.exceptions import (
TooManyFieldsSent,
)
from django.core.files import uploadhandler
from django.http.multipartparser import MultiPartParser, MultiPartParserError
from django.http.multipartparser import (
MultiPartParser,
MultiPartParserError,
TooManyFilesSent,
)
from django.utils.datastructures import (
CaseInsensitiveMapping,
ImmutableList,
@ -382,7 +386,7 @@ class HttpRequest:
data = self
try:
self._post, self._files = self.parse_file_upload(self.META, data)
except MultiPartParserError:
except (MultiPartParserError, TooManyFilesSent):
# An error occurred while parsing POST data. Since when
# formatting the error the request handler might access
# self.POST, set self._post and self._file to prevent

5
docs/ref/exceptions.txt
View File

@ -95,12 +95,17 @@ Django core exception classes are defined in ``django.core.exceptions``.
* ``SuspiciousMultipartForm``
* ``SuspiciousSession``
* ``TooManyFieldsSent``
* ``TooManyFilesSent``
If a ``SuspiciousOperation`` exception reaches the ASGI/WSGI handler level
it is logged at the ``Error`` level and results in
a :class:`~django.http.HttpResponseBadRequest`. See the :doc:`logging
documentation </topics/logging/>` for more information.
.. versionchanged:: 3.2.18
``SuspiciousOperation`` is raised when too many files are submitted.
``PermissionDenied``
--------------------

23
docs/ref/settings.txt
View File

@ -1062,6 +1062,28 @@ could be used as a denial-of-service attack vector if left unchecked. Since web
servers don't typically perform deep request inspection, it's not possible to
perform a similar check at that level.
.. setting:: DATA_UPLOAD_MAX_NUMBER_FILES
``DATA_UPLOAD_MAX_NUMBER_FILES``
--------------------------------
.. versionadded:: 3.2.18
Default: ``100``
The maximum number of files that may be received via POST in a
``multipart/form-data`` encoded request before a
:exc:`~django.core.exceptions.SuspiciousOperation` (``TooManyFiles``) is
raised. You can set this to ``None`` to disable the check. Applications that
are expected to receive an unusually large number of file fields should tune
this setting.
The number of accepted files is correlated to the amount of time and memory
needed to process the request. Large requests could be used as a
denial-of-service attack vector if left unchecked. Since web servers don't
typically perform deep request inspection, it's not possible to perform a
similar check at that level.
.. setting:: DATABASE_ROUTERS
``DATABASE_ROUTERS``
@ -3685,6 +3707,7 @@ HTTP
----
* :setting:`DATA_UPLOAD_MAX_MEMORY_SIZE`
* :setting:`DATA_UPLOAD_MAX_NUMBER_FIELDS`
* :setting:`DATA_UPLOAD_MAX_NUMBER_FILES`
* :setting:`DEFAULT_CHARSET`
* :setting:`DISALLOWED_USER_AGENTS`
* :setting:`FORCE_SCRIPT_NAME`

10
docs/releases/3.2.18.txt
View File

@ -6,4 +6,12 @@ Django 3.2.18 release notes
Django 3.2.18 fixes a security issue with severity "moderate" in 3.2.17.
...
CVE-2023-24580: Potential denial-of-service vulnerability in file uploads
=========================================================================
Passing certain inputs to multipart forms could result in too many open files
or memory exhaustion, and provided a potential vector for a denial-of-service
attack.
The number of files parts parsed is now limited via the new
:setting:`DATA_UPLOAD_MAX_NUMBER_FILES` setting.

10
docs/releases/4.0.10.txt
View File

@ -6,4 +6,12 @@ Django 4.0.10 release notes
Django 4.0.10 fixes a security issue with severity "moderate" in 4.0.9.
...
CVE-2023-24580: Potential denial-of-service vulnerability in file uploads
=========================================================================
Passing certain inputs to multipart forms could result in too many open files
or memory exhaustion, and provided a potential vector for a denial-of-service
attack.
The number of files parts parsed is now limited via the new
:setting:`DATA_UPLOAD_MAX_NUMBER_FILES` setting.

14
docs/releases/4.1.7.txt
View File

@ -4,10 +4,18 @@ Django 4.1.7 release notes
*February 14, 2023*
Django 4.1.7 fixes a security issue with severity "moderate" and several bugs
in 4.1.6.
Django 4.1.7 fixes a security issue with severity "moderate" and a bug in
4.1.6.
...
CVE-2023-24580: Potential denial-of-service vulnerability in file uploads
=========================================================================
Passing certain inputs to multipart forms could result in too many open files
or memory exhaustion, and provided a potential vector for a denial-of-service
attack.
The number of files parts parsed is now limited via the new
:setting:`DATA_UPLOAD_MAX_NUMBER_FILES` setting.
Bugfixes
========

31
tests/handlers/test_exception.py
View File

@ -1,6 +1,11 @@
from django.core.handlers.wsgi import WSGIHandler
from django.test import SimpleTestCase, override_settings
from django.test.client import FakePayload
from django.test.client import (
BOUNDARY,
MULTIPART_CONTENT,
FakePayload,
encode_multipart,
)
class ExceptionHandlerTests(SimpleTestCase):
@ -24,3 +29,27 @@ class ExceptionHandlerTests(SimpleTestCase):
def test_data_upload_max_number_fields_exceeded(self):
response = WSGIHandler()(self.get_suspicious_environ(), lambda *a, **k: None)
self.assertEqual(response.status_code, 400)
@override_settings(DATA_UPLOAD_MAX_NUMBER_FILES=2)
def test_data_upload_max_number_files_exceeded(self):
payload = FakePayload(
encode_multipart(
BOUNDARY,
{
"a.txt": "Hello World!",
"b.txt": "Hello Django!",
"c.txt": "Hello Python!",
},
)
)
environ = {
"REQUEST_METHOD": "POST",
"CONTENT_TYPE": MULTIPART_CONTENT,
"CONTENT_LENGTH": len(payload),
"wsgi.input": payload,
"SERVER_NAME": "test",
"SERVER_PORT": "8000",
}
response = WSGIHandler()(environ, lambda *a, **k: None)
self.assertEqual(response.status_code, 400)

55
tests/requests_tests/test_data_upload_settings.py
View File

@ -1,6 +1,10 @@
from io import BytesIO
from django.core.exceptions import RequestDataTooBig, TooManyFieldsSent
from django.core.exceptions import (
RequestDataTooBig,
TooManyFieldsSent,
TooManyFilesSent,
)
from django.core.handlers.wsgi import WSGIRequest
from django.test import SimpleTestCase
from django.test.client import FakePayload
@ -8,6 +12,9 @@ from django.test.client import FakePayload
TOO_MANY_FIELDS_MSG = (
"The number of GET/POST parameters exceeded settings.DATA_UPLOAD_MAX_NUMBER_FIELDS."
)
TOO_MANY_FILES_MSG = (
"The number of files exceeded settings.DATA_UPLOAD_MAX_NUMBER_FILES."
)
TOO_MUCH_DATA_MSG = "Request body exceeded settings.DATA_UPLOAD_MAX_MEMORY_SIZE."
@ -191,6 +198,52 @@ class DataUploadMaxNumberOfFieldsMultipartPost(SimpleTestCase):
self.request._load_post_and_files()
class DataUploadMaxNumberOfFilesMultipartPost(SimpleTestCase):
def setUp(self):
payload = FakePayload(
"\r\n".join(
[
"--boundary",
(
'Content-Disposition: form-data; name="name1"; '
'filename="name1.txt"'
),
"",
"value1",
"--boundary",
(
'Content-Disposition: form-data; name="name2"; '
'filename="name2.txt"'
),
"",
"value2",
"--boundary--",
]
)
)
self.request = WSGIRequest(
{
"REQUEST_METHOD": "POST",
"CONTENT_TYPE": "multipart/form-data; boundary=boundary",
"CONTENT_LENGTH": len(payload),
"wsgi.input": payload,
}
)
def test_number_exceeded(self):
with self.settings(DATA_UPLOAD_MAX_NUMBER_FILES=1):
with self.assertRaisesMessage(TooManyFilesSent, TOO_MANY_FILES_MSG):
self.request._load_post_and_files()
def test_number_not_exceeded(self):
with self.settings(DATA_UPLOAD_MAX_NUMBER_FILES=2):
self.request._load_post_and_files()
def test_no_limit(self):
with self.settings(DATA_UPLOAD_MAX_NUMBER_FILES=None):
self.request._load_post_and_files()
class DataUploadMaxNumberOfFieldsFormPost(SimpleTestCase):
def setUp(self):
payload = FakePayload("\r\n".join(["a=1&a=2&a=3", ""]))