diff --git a/dojo/authorization/api_permissions.py b/dojo/authorization/api_permissions.py index 24e5fdd37f2..3a801e6dc35 100644 --- a/dojo/authorization/api_permissions.py +++ b/dojo/authorization/api_permissions.py @@ -1,5 +1,6 @@ from django.conf import settings +from django.core.exceptions import RequestDataTooBig, TooManyFieldsSent from django.db.models import Model from django.shortcuts import get_object_or_404 from rest_framework import permissions, serializers @@ -544,6 +545,19 @@ def has_permission(self, request, view): converted_dict["product_type"] = auto_create.get_target_product_type_if_exists(**converted_dict) converted_dict["product"] = auto_create.get_target_product_if_exists(**converted_dict) converted_dict["engagement"] = auto_create.get_target_engagement_if_exists(**converted_dict) + except (TooManyFieldsSent, RequestDataTooBig) as e: + # A very large scan import (too many form fields, or a body over the size limit) + # trips Django's DATA_UPLOAD_MAX_NUMBER_FIELDS / DATA_UPLOAD_MAX_MEMORY_SIZE guard + # while this permission check parses request.data. Surface it as a clear client + # error instead of letting the SuspiciousOperation escape as an opaque 400 that + # also pages on-call via error reporting. + msg = ( + "The scan import request exceeded the server's upload limits " + "(too many form fields, or the request body is too large). Reduce the " + "number of fields in the request, or ask your administrator to increase " + "DD_DATA_UPLOAD_MAX_NUMBER_FIELDS / DD_DATA_UPLOAD_MAX_MEMORY_SIZE." + ) + raise ValidationError(msg) from e except (ValueError, TypeError) as e: # Raise an explicit drf exception here raise ValidationError(e) @@ -604,6 +618,19 @@ def has_permission(self, request, view): product = auto_create.get_target_product_if_exists(**converted_dict) if not product: product = auto_create.get_target_product_by_id_if_exists(**converted_dict) + except (TooManyFieldsSent, RequestDataTooBig) as e: + # A very large scan import (too many form fields, or a body over the size limit) + # trips Django's DATA_UPLOAD_MAX_NUMBER_FIELDS / DATA_UPLOAD_MAX_MEMORY_SIZE guard + # while this permission check parses request.data. Surface it as a clear client + # error instead of letting the SuspiciousOperation escape as an opaque 400 that + # also pages on-call via error reporting. + msg = ( + "The scan import request exceeded the server's upload limits " + "(too many form fields, or the request body is too large). Reduce the " + "number of fields in the request, or ask your administrator to increase " + "DD_DATA_UPLOAD_MAX_NUMBER_FIELDS / DD_DATA_UPLOAD_MAX_MEMORY_SIZE." + ) + raise ValidationError(msg) from e except (ValueError, TypeError) as e: # Raise an explicit drf exception here raise ValidationError(e) @@ -725,6 +752,19 @@ def has_permission(self, request, view): converted_dict["product"] = auto_create.get_target_product_if_exists(**converted_dict) converted_dict["engagement"] = auto_create.get_target_engagement_if_exists(**converted_dict) converted_dict["test"] = auto_create.get_target_test_if_exists(**converted_dict) + except (TooManyFieldsSent, RequestDataTooBig) as e: + # A very large scan import (too many form fields, or a body over the size limit) + # trips Django's DATA_UPLOAD_MAX_NUMBER_FIELDS / DATA_UPLOAD_MAX_MEMORY_SIZE guard + # while this permission check parses request.data. Surface it as a clear client + # error instead of letting the SuspiciousOperation escape as an opaque 400 that + # also pages on-call via error reporting. + msg = ( + "The scan import request exceeded the server's upload limits " + "(too many form fields, or the request body is too large). Reduce the " + "number of fields in the request, or ask your administrator to increase " + "DD_DATA_UPLOAD_MAX_NUMBER_FIELDS / DD_DATA_UPLOAD_MAX_MEMORY_SIZE." + ) + raise ValidationError(msg) from e except (ValueError, TypeError) as e: # Raise an explicit drf exception here raise ValidationError(e) diff --git a/dojo/settings/settings.dist.py b/dojo/settings/settings.dist.py index e9293884764..3a3f9ff217b 100644 --- a/dojo/settings/settings.dist.py +++ b/dojo/settings/settings.dist.py @@ -157,6 +157,7 @@ DD_SECRET_KEY=(str, ""), DD_CREDENTIAL_AES_256_KEY=(str, "."), DD_DATA_UPLOAD_MAX_MEMORY_SIZE=(int, 8388608), # Max post size set to 8mb + DD_DATA_UPLOAD_MAX_NUMBER_FIELDS=(int, 10240), # Max number of GET/POST parameters in a request DD_MAX_ZIP_MEMBERS=(int, 1000), DD_MAX_ZIP_MEMBER_SIZE=(int, 512 * 1024 * 1024), # 512 MB per member (uncompressed) DD_MAX_ZIP_TOTAL_SIZE=(int, 1 * 1024 * 1024 * 1024), # 1 GB total (uncompressed) @@ -2096,7 +2097,9 @@ def generate_url(scheme, double_slashes, user, password, host, port, path, param DEFAULT_EXCEPTION_REPORTER_FILTER = "dojo.settings.exception_filter.CustomExceptionReporterFilter" # Issue on benchmark : "The number of GET/POST parameters exceeded settings.DATA_UPLOAD_MAX_NUMBER_FIELD S" -DATA_UPLOAD_MAX_NUMBER_FIELDS = 10240 +# Configurable so operators can raise it for instances that legitimately submit very large +# scan imports (many form fields), mirroring DD_DATA_UPLOAD_MAX_MEMORY_SIZE above. +DATA_UPLOAD_MAX_NUMBER_FIELDS = env("DD_DATA_UPLOAD_MAX_NUMBER_FIELDS") # Maximum size of a scan file in MB SCAN_FILE_MAX_SIZE = env("DD_SCAN_FILE_MAX_SIZE") diff --git a/unittests/test_import_permission_upload_limits.py b/unittests/test_import_permission_upload_limits.py new file mode 100644 index 00000000000..4d734e3bf57 --- /dev/null +++ b/unittests/test_import_permission_upload_limits.py @@ -0,0 +1,70 @@ +""" +Regression tests for scan-import permission checks under upload limits. + +The import/reimport permission classes parse ``request.data`` inside their +``has_permission`` to resolve the target product/engagement/test before the +serializer runs. A very large scan import (many form fields) makes Django's +multipart parser raise ``TooManyFieldsSent`` (a ``SuspiciousOperation``) while +``request.data`` is evaluated. That exception used to escape the permission +check as an opaque error and generate on-call noise; it must instead surface as +a clean DRF ``ValidationError`` (HTTP 400) with an actionable message. + +The limit itself (``DATA_UPLOAD_MAX_NUMBER_FIELDS``) is now configurable via the +``DD_DATA_UPLOAD_MAX_NUMBER_FIELDS`` environment variable so operators can raise +it for instances that legitimately submit very large imports. +""" +from django.conf import settings +from django.core.exceptions import TooManyFieldsSent +from django.test import SimpleTestCase, override_settings +from rest_framework.exceptions import ValidationError +from rest_framework.parsers import FormParser, MultiPartParser +from rest_framework.request import Request +from rest_framework.test import APIRequestFactory + +from dojo.authorization.api_permissions import ( + UserHasImportPermission, + UserHasMetaImportPermission, + UserHasReimportPermission, +) + +IMPORT_PERMISSION_CLASSES = ( + UserHasImportPermission, + UserHasMetaImportPermission, + UserHasReimportPermission, +) + + +class ImportPermissionUploadLimitsTest(SimpleTestCase): + def _multipart_request(self, field_count: int) -> Request: + payload = {f"field_{i}": "x" for i in range(field_count)} + django_request = APIRequestFactory().post( + "/api/v2/import-scan/", payload, format="multipart", + ) + return Request(django_request, parsers=[MultiPartParser(), FormParser()]) + + @override_settings(DATA_UPLOAD_MAX_NUMBER_FIELDS=5) + def test_too_many_fields_raises_validation_error(self): + # Without the fix Django's TooManyFieldsSent escapes the permission check; + # with the fix each import/reimport permission raises a DRF ValidationError. + for permission_class in IMPORT_PERMISSION_CLASSES: + with self.subTest(permission=permission_class.__name__): + request = self._multipart_request(field_count=20) + with self.assertRaises(ValidationError) as ctx: + permission_class().has_permission(request, view=None) + self.assertIn("upload limits", str(ctx.exception).lower()) + + @override_settings(DATA_UPLOAD_MAX_NUMBER_FIELDS=5) + def test_request_within_limit_parses_without_size_error(self): + # A request under the limit parses normally (no SuspiciousOperation). + request = self._multipart_request(field_count=3) + try: + parsed = request.data + except TooManyFieldsSent: # pragma: no cover - would mean the guard misfired + self.fail("request under the field limit must not raise TooManyFieldsSent") + self.assertEqual(sorted(parsed.keys()), ["field_0", "field_1", "field_2"]) + + +class DataUploadMaxNumberFieldsSettingTest(SimpleTestCase): + def test_default_value(self): + # Default preserved while the value is now sourced from the environment. + self.assertEqual(settings.DATA_UPLOAD_MAX_NUMBER_FIELDS, 10240)