From 985e0b86df7a1e8d0867804b5d26fb0c3cf6ef95 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Tue, 14 Apr 2026 15:26:50 -0600 Subject: [PATCH 1/4] Fix reimport-scan API authorization bypass via conflicting identifiers Validate that ID-resolved objects (test, engagement) are consistent with name-based identifiers (product_name, engagement_name) in both the permission check layer and the AutoCreateContextManager resolution layer. This prevents an attacker from passing their own engagement/test ID to satisfy the permission check while using name-based fields to target a victim's product. Co-Authored-By: Claude Opus 4.6 (1M context) --- dojo/api_v2/permissions.py | 18 +++++- dojo/importers/auto_create_context.py | 6 ++ unittests/test_rest_framework.py | 79 +++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 3 deletions(-) diff --git a/dojo/api_v2/permissions.py b/dojo/api_v2/permissions.py index 807bbcf7b2a..7ec44dd49e6 100644 --- a/dojo/api_v2/permissions.py +++ b/dojo/api_v2/permissions.py @@ -473,7 +473,10 @@ def has_permission(self, request, view): # Raise an explicit drf exception here raise ValidationError(e) if engagement := converted_dict.get("engagement"): - # existing engagement, nothing special to check + # Validate the resolved engagement's parent chain matches any provided names + if (product_name := converted_dict.get("product_name")) and engagement.product.name != product_name: + msg = f'The resolved engagement is associated with product "{engagement.product.name}", not with product "{product_name}"' + raise ValidationError(msg) return user_has_permission( request.user, engagement, Permissions.Import_Scan_Result, ) @@ -774,7 +777,13 @@ def has_permission(self, request, view): raise ValidationError(e) if test := converted_dict.get("test"): - # existing test, nothing special to check + # Validate the resolved test's parent chain matches any provided names + if (product_name := converted_dict.get("product_name")) and test.engagement.product.name != product_name: + msg = f'The resolved test is associated with product "{test.engagement.product.name}", not with product "{product_name}"' + raise ValidationError(msg) + if (engagement_name := converted_dict.get("engagement_name")) and test.engagement.name != engagement_name: + msg = f'The resolved test is associated with engagement "{test.engagement.name}", not with engagement "{engagement_name}"' + raise ValidationError(msg) return user_has_permission( request.user, test, Permissions.Import_Scan_Result, ) @@ -1181,7 +1190,10 @@ def check_auto_create_permission( raise ValidationError(msg) if engagement: - # existing engagement, nothing special to check + # Validate the resolved engagement's parent chain matches any provided names + if product is not None and engagement.product_id != product.id: + msg = f'The resolved engagement is associated with product "{engagement.product.name}", not with product "{product_name}"' + raise ValidationError(msg) return user_has_permission( user, engagement, Permissions.Import_Scan_Result, ) diff --git a/dojo/importers/auto_create_context.py b/dojo/importers/auto_create_context.py index 26d37ae65b0..00611b22dab 100644 --- a/dojo/importers/auto_create_context.py +++ b/dojo/importers/auto_create_context.py @@ -181,6 +181,9 @@ def get_target_engagement_if_exists( """ if engagement := get_object_or_none(Engagement, pk=engagement_id): logger.debug("Using existing engagement by id: %s", engagement_id) + if product is not None and engagement.product_id != product.id: + msg = f"Engagement \"{engagement_id}\" does not belong to product \"{product}\"" + raise ValueError(msg) return engagement # if there's no product, then for sure there's no engagement either if product is None: @@ -203,6 +206,9 @@ def get_target_test_if_exists( """ if test := get_object_or_none(Test, pk=test_id): logger.debug("Using existing Test by id: %s", test_id) + if engagement is not None and test.engagement_id != engagement.id: + msg = f"Test \"{test_id}\" does not belong to engagement \"{engagement}\"" + raise ValueError(msg) return test # If the engagement is not supplied, we cannot do anything if not engagement: diff --git a/unittests/test_rest_framework.py b/unittests/test_rest_framework.py index 95807d4c536..94f5da9f887 100644 --- a/unittests/test_rest_framework.py +++ b/unittests/test_rest_framework.py @@ -3429,6 +3429,85 @@ def test_create_not_authorized_product_name_engagement_name_scan_type_title(self importer_mock.assert_not_called() reimporter_mock.assert_not_called() + # Security tests: verify that conflicting ID-based and name-based identifiers are rejected + + @patch("dojo.importers.default_reimporter.DefaultReImporter.process_scan") + @patch("dojo.importers.default_importer.DefaultImporter.process_scan") + def test_reimport_with_engagement_id_mismatched_product_name_is_rejected(self, importer_mock, reimporter_mock): + """Sending engagement ID from one product with product_name from another must be rejected.""" + importer_mock.return_value = IMPORTER_MOCK_RETURN_VALUE + reimporter_mock.return_value = REIMPORTER_MOCK_RETURN_VALUE + + with Path("tests/zap_sample.xml").open(encoding="utf-8") as testfile: + payload = { + "minimum_severity": "Low", + "active": True, + "verified": True, + "scan_type": "ZAP Scan", + "file": testfile, + # Engagement 1 belongs to Product 2 ("Security How-to") + "engagement": 1, + # But product_name points to Product 1 ("Python How-to") + "product_name": "Python How-to", + "engagement_name": "April monthly engagement", + "version": "1.0.0", + } + response = self.client.post(self.url, payload) + self.assertEqual(400, response.status_code, response.content[:1000]) + importer_mock.assert_not_called() + reimporter_mock.assert_not_called() + + @patch("dojo.importers.default_reimporter.DefaultReImporter.process_scan") + @patch("dojo.importers.default_importer.DefaultImporter.process_scan") + def test_reimport_with_test_id_mismatched_product_name_is_rejected(self, importer_mock, reimporter_mock): + """Sending test ID from one product with product_name from another must be rejected.""" + importer_mock.return_value = IMPORTER_MOCK_RETURN_VALUE + reimporter_mock.return_value = REIMPORTER_MOCK_RETURN_VALUE + + with Path("tests/zap_sample.xml").open(encoding="utf-8") as testfile: + payload = { + "minimum_severity": "Low", + "active": True, + "verified": True, + "scan_type": "ZAP Scan", + "file": testfile, + # Test 3 belongs to Engagement 1 -> Product 2 ("Security How-to") + "test": 3, + # But product_name points to Product 1 ("Python How-to") + "product_name": "Python How-to", + "version": "1.0.0", + } + response = self.client.post(self.url, payload) + self.assertEqual(400, response.status_code, response.content[:1000]) + importer_mock.assert_not_called() + reimporter_mock.assert_not_called() + + @patch("dojo.importers.default_reimporter.DefaultReImporter.process_scan") + @patch("dojo.importers.default_importer.DefaultImporter.process_scan") + def test_reimport_with_test_id_mismatched_engagement_name_is_rejected(self, importer_mock, reimporter_mock): + """Sending test ID from one engagement with engagement_name from another must be rejected.""" + importer_mock.return_value = IMPORTER_MOCK_RETURN_VALUE + reimporter_mock.return_value = REIMPORTER_MOCK_RETURN_VALUE + + with Path("tests/zap_sample.xml").open(encoding="utf-8") as testfile: + payload = { + "minimum_severity": "Low", + "active": True, + "verified": True, + "scan_type": "ZAP Scan", + "file": testfile, + # Test 3 belongs to Engagement 1 ("1st Quarter Engagement") + "test": 3, + # But engagement_name points to a different engagement + "product_name": "Security How-to", + "engagement_name": "April monthly engagement", + "version": "1.0.0", + } + response = self.client.post(self.url, payload) + self.assertEqual(400, response.status_code, response.content[:1000]) + importer_mock.assert_not_called() + reimporter_mock.assert_not_called() + @versioned_fixtures class ProductTypeTest(BaseClass.BaseClassTest): From 92cd890469f6a90dc12c4c3f95641651cc41486a Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Tue, 14 Apr 2026 15:32:34 -0600 Subject: [PATCH 2/4] Use ID-based comparisons and add engagement_name check to import - Switch permission checks to use ID comparisons (product_id, engagement_id) where resolved objects are available, with name fallback for unresolved cases - Add engagement_name validation to UserHasImportPermission (was missing) - Fix ruff string quoting in auto_create_context.py Co-Authored-By: Claude Opus 4.6 (1M context) --- dojo/api_v2/permissions.py | 22 ++++++++++++++++------ dojo/importers/auto_create_context.py | 4 ++-- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/dojo/api_v2/permissions.py b/dojo/api_v2/permissions.py index 7ec44dd49e6..456ca2c5eab 100644 --- a/dojo/api_v2/permissions.py +++ b/dojo/api_v2/permissions.py @@ -473,9 +473,12 @@ def has_permission(self, request, view): # Raise an explicit drf exception here raise ValidationError(e) if engagement := converted_dict.get("engagement"): - # Validate the resolved engagement's parent chain matches any provided names - if (product_name := converted_dict.get("product_name")) and engagement.product.name != product_name: - msg = f'The resolved engagement is associated with product "{engagement.product.name}", not with product "{product_name}"' + # Validate the resolved engagement's parent chain matches any provided identifiers + if (product := converted_dict.get("product")) and engagement.product_id != product.id: + msg = f'The resolved engagement is associated with product "{engagement.product.name}", not with product "{converted_dict.get("product_name")}"' + raise ValidationError(msg) + if (engagement_name := converted_dict.get("engagement_name")) and engagement.name != engagement_name: + msg = f'The resolved engagement is named "{engagement.name}", not "{engagement_name}"' raise ValidationError(msg) return user_has_permission( request.user, engagement, Permissions.Import_Scan_Result, @@ -777,11 +780,18 @@ def has_permission(self, request, view): raise ValidationError(e) if test := converted_dict.get("test"): - # Validate the resolved test's parent chain matches any provided names - if (product_name := converted_dict.get("product_name")) and test.engagement.product.name != product_name: + # Validate the resolved test's parent chain matches any provided identifiers + if (product := converted_dict.get("product")) and test.engagement.product_id != product.id: + msg = f'The resolved test is associated with product "{test.engagement.product.name}", not with product "{converted_dict.get("product_name")}"' + raise ValidationError(msg) + if (engagement := converted_dict.get("engagement")) and test.engagement_id != engagement.id: + msg = f'The resolved test is associated with engagement "{test.engagement.name}", not with engagement "{converted_dict.get("engagement_name")}"' + raise ValidationError(msg) + # Also validate by name when the objects were not resolved (e.g. names that match no existing record) + if not converted_dict.get("product") and (product_name := converted_dict.get("product_name")) and test.engagement.product.name != product_name: msg = f'The resolved test is associated with product "{test.engagement.product.name}", not with product "{product_name}"' raise ValidationError(msg) - if (engagement_name := converted_dict.get("engagement_name")) and test.engagement.name != engagement_name: + if not converted_dict.get("engagement") and (engagement_name := converted_dict.get("engagement_name")) and test.engagement.name != engagement_name: msg = f'The resolved test is associated with engagement "{test.engagement.name}", not with engagement "{engagement_name}"' raise ValidationError(msg) return user_has_permission( diff --git a/dojo/importers/auto_create_context.py b/dojo/importers/auto_create_context.py index 00611b22dab..9e9103edd41 100644 --- a/dojo/importers/auto_create_context.py +++ b/dojo/importers/auto_create_context.py @@ -182,7 +182,7 @@ def get_target_engagement_if_exists( if engagement := get_object_or_none(Engagement, pk=engagement_id): logger.debug("Using existing engagement by id: %s", engagement_id) if product is not None and engagement.product_id != product.id: - msg = f"Engagement \"{engagement_id}\" does not belong to product \"{product}\"" + msg = f'Engagement "{engagement_id}" does not belong to product "{product}"' raise ValueError(msg) return engagement # if there's no product, then for sure there's no engagement either @@ -207,7 +207,7 @@ def get_target_test_if_exists( if test := get_object_or_none(Test, pk=test_id): logger.debug("Using existing Test by id: %s", test_id) if engagement is not None and test.engagement_id != engagement.id: - msg = f"Test \"{test_id}\" does not belong to engagement \"{engagement}\"" + msg = f'Test "{test_id}" does not belong to engagement "{engagement}"' raise ValueError(msg) return test # If the engagement is not supplied, we cannot do anything From 6dd70f880d155eeb29a2b532d2d15d45bed17acd Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Tue, 14 Apr 2026 16:10:33 -0600 Subject: [PATCH 3/4] Strip undeclared engagement field in reimport permission check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The engagement field is not declared on ReImportScanSerializer and gets stripped during validation. The permission check must also strip it so it resolves targets the same way execution does — by name, not by a stale engagement ID from request.data. Update test to verify the engagement param is ignored and permission is checked against the name-resolved target. Co-Authored-By: Claude Opus 4.6 (1M context) --- dojo/api_v2/permissions.py | 5 +++++ unittests/test_rest_framework.py | 22 ++++++++++++++++------ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/dojo/api_v2/permissions.py b/dojo/api_v2/permissions.py index 456ca2c5eab..acfe57cb016 100644 --- a/dojo/api_v2/permissions.py +++ b/dojo/api_v2/permissions.py @@ -770,6 +770,11 @@ def has_permission(self, request, view): try: converted_dict = auto_create.convert_querydict_to_dict(request.data) auto_create.process_import_meta_data_from_dict(converted_dict) + # engagement is not a declared field on ReImportScanSerializer and will be + # stripped during validation — don't use it in the permission check either, + # so the permission check resolves targets the same way execution does + converted_dict.pop("engagement", None) + converted_dict.pop("engagement_id", None) # Get an existing product 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) diff --git a/unittests/test_rest_framework.py b/unittests/test_rest_framework.py index 94f5da9f887..7199e08c126 100644 --- a/unittests/test_rest_framework.py +++ b/unittests/test_rest_framework.py @@ -3433,8 +3433,13 @@ def test_create_not_authorized_product_name_engagement_name_scan_type_title(self @patch("dojo.importers.default_reimporter.DefaultReImporter.process_scan") @patch("dojo.importers.default_importer.DefaultImporter.process_scan") - def test_reimport_with_engagement_id_mismatched_product_name_is_rejected(self, importer_mock, reimporter_mock): - """Sending engagement ID from one product with product_name from another must be rejected.""" + @patch("dojo.api_v2.permissions.user_has_permission") + def test_reimport_engagement_param_ignored_permission_checked_on_name_resolved_target(self, mock, importer_mock, reimporter_mock): + """ + Engagement is not a declared field on ReImportScanSerializer — verify + the permission check uses the name-resolved target, not the engagement param. + """ + mock.return_value = False importer_mock.return_value = IMPORTER_MOCK_RETURN_VALUE reimporter_mock.return_value = REIMPORTER_MOCK_RETURN_VALUE @@ -3445,15 +3450,20 @@ def test_reimport_with_engagement_id_mismatched_product_name_is_rejected(self, i "verified": True, "scan_type": "ZAP Scan", "file": testfile, - # Engagement 1 belongs to Product 2 ("Security How-to") + # engagement=1 belongs to Product 2 Engagement 1, but it should be ignored "engagement": 1, - # But product_name points to Product 1 ("Python How-to") - "product_name": "Python How-to", + # These names resolve to Product 2's Engagement 4 -> Test 4 + "product_name": "Security How-to", "engagement_name": "April monthly engagement", "version": "1.0.0", } response = self.client.post(self.url, payload) - self.assertEqual(400, response.status_code, response.content[:1000]) + self.assertEqual(403, response.status_code, response.content[:1000]) + # Permission must be checked on name-resolved Test 4 (in Engagement 4), + # NOT on Test 3 (which belongs to the engagement=1 param) + mock.assert_called_with(User.objects.get(username="admin"), + Test.objects.get(id=4), + Permissions.Import_Scan_Result) importer_mock.assert_not_called() reimporter_mock.assert_not_called() From 70c2c19a66787d61436b4ba363f4cd830dc34719 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Thu, 16 Apr 2026 09:18:28 -0600 Subject: [PATCH 4/4] Fix information disclosure in conflict validation error messages Replace error messages that leaked resolved object names (product names, engagement names) with generic messages. An attacker could enumerate object names by sending conflicting ID-based and name-based identifiers and reading the detailed error responses. Co-Authored-By: Claude Opus 4.6 (1M context) --- dojo/api_v2/permissions.py | 14 +++++++------- dojo/importers/auto_create_context.py | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/dojo/api_v2/permissions.py b/dojo/api_v2/permissions.py index acfe57cb016..10e012ede5b 100644 --- a/dojo/api_v2/permissions.py +++ b/dojo/api_v2/permissions.py @@ -475,10 +475,10 @@ def has_permission(self, request, view): if engagement := converted_dict.get("engagement"): # Validate the resolved engagement's parent chain matches any provided identifiers if (product := converted_dict.get("product")) and engagement.product_id != product.id: - msg = f'The resolved engagement is associated with product "{engagement.product.name}", not with product "{converted_dict.get("product_name")}"' + msg = "The provided identifiers are inconsistent — the engagement does not belong to the specified product." raise ValidationError(msg) if (engagement_name := converted_dict.get("engagement_name")) and engagement.name != engagement_name: - msg = f'The resolved engagement is named "{engagement.name}", not "{engagement_name}"' + msg = "The provided identifiers are inconsistent — the engagement name does not match the specified engagement." raise ValidationError(msg) return user_has_permission( request.user, engagement, Permissions.Import_Scan_Result, @@ -787,17 +787,17 @@ def has_permission(self, request, view): if test := converted_dict.get("test"): # Validate the resolved test's parent chain matches any provided identifiers if (product := converted_dict.get("product")) and test.engagement.product_id != product.id: - msg = f'The resolved test is associated with product "{test.engagement.product.name}", not with product "{converted_dict.get("product_name")}"' + msg = "The provided identifiers are inconsistent — the test does not belong to the specified product." raise ValidationError(msg) if (engagement := converted_dict.get("engagement")) and test.engagement_id != engagement.id: - msg = f'The resolved test is associated with engagement "{test.engagement.name}", not with engagement "{converted_dict.get("engagement_name")}"' + msg = "The provided identifiers are inconsistent — the test does not belong to the specified engagement." raise ValidationError(msg) # Also validate by name when the objects were not resolved (e.g. names that match no existing record) if not converted_dict.get("product") and (product_name := converted_dict.get("product_name")) and test.engagement.product.name != product_name: - msg = f'The resolved test is associated with product "{test.engagement.product.name}", not with product "{product_name}"' + msg = "The provided identifiers are inconsistent — the test does not belong to the specified product." raise ValidationError(msg) if not converted_dict.get("engagement") and (engagement_name := converted_dict.get("engagement_name")) and test.engagement.name != engagement_name: - msg = f'The resolved test is associated with engagement "{test.engagement.name}", not with engagement "{engagement_name}"' + msg = "The provided identifiers are inconsistent — the test does not belong to the specified engagement." raise ValidationError(msg) return user_has_permission( request.user, test, Permissions.Import_Scan_Result, @@ -1207,7 +1207,7 @@ def check_auto_create_permission( if engagement: # Validate the resolved engagement's parent chain matches any provided names if product is not None and engagement.product_id != product.id: - msg = f'The resolved engagement is associated with product "{engagement.product.name}", not with product "{product_name}"' + msg = "The provided identifiers are inconsistent — the engagement does not belong to the specified product." raise ValidationError(msg) return user_has_permission( user, engagement, Permissions.Import_Scan_Result, diff --git a/dojo/importers/auto_create_context.py b/dojo/importers/auto_create_context.py index 9e9103edd41..916bbea056e 100644 --- a/dojo/importers/auto_create_context.py +++ b/dojo/importers/auto_create_context.py @@ -182,7 +182,7 @@ def get_target_engagement_if_exists( if engagement := get_object_or_none(Engagement, pk=engagement_id): logger.debug("Using existing engagement by id: %s", engagement_id) if product is not None and engagement.product_id != product.id: - msg = f'Engagement "{engagement_id}" does not belong to product "{product}"' + msg = "The provided identifiers are inconsistent — the engagement does not belong to the specified product." raise ValueError(msg) return engagement # if there's no product, then for sure there's no engagement either @@ -207,7 +207,7 @@ def get_target_test_if_exists( if test := get_object_or_none(Test, pk=test_id): logger.debug("Using existing Test by id: %s", test_id) if engagement is not None and test.engagement_id != engagement.id: - msg = f'Test "{test_id}" does not belong to engagement "{engagement}"' + msg = "The provided identifiers are inconsistent — the test does not belong to the specified engagement." raise ValueError(msg) return test # If the engagement is not supplied, we cannot do anything