[4.0.x] Fixed CVE-2023-24580 -- Prevented DoS with too many uploaded files.

Thanks to Jakob Ackermann for the report.

2 files changed, 57 insertions, 15 deletions

diff --git a/django/http/multipartparser.py b/django/http/multipartparser.py
index 77a4b1f140..64f8c6d4cf 100644
--- a/django/http/multipartparser.py
+++ b/django/http/multipartparser.py
@@ -16,6 +16,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:
@@ -109,6 +111,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.
@@ -154,6 +172,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.
@@ -169,6 +189,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()
@@ -181,17 +215,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 = (
@@ -226,6 +249,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:
@@ -303,8 +336,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:
diff --git a/django/http/request.py b/django/http/request.py
index f32d57ba67..a32953e86e 100644
--- a/django/http/request.py
+++ b/django/http/request.py
@@ -14,7 +14,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,
@@ -367,7 +371,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