Skip to content

Commit 985e0b8

Browse files
Maffoochclaude
andcommitted
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) <noreply@anthropic.com>
1 parent 9f92409 commit 985e0b8

File tree

3 files changed

+100
-3
lines changed

3 files changed

+100
-3
lines changed

dojo/api_v2/permissions.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,10 @@ def has_permission(self, request, view):
473473
# Raise an explicit drf exception here
474474
raise ValidationError(e)
475475
if engagement := converted_dict.get("engagement"):
476-
# existing engagement, nothing special to check
476+
# Validate the resolved engagement's parent chain matches any provided names
477+
if (product_name := converted_dict.get("product_name")) and engagement.product.name != product_name:
478+
msg = f'The resolved engagement is associated with product "{engagement.product.name}", not with product "{product_name}"'
479+
raise ValidationError(msg)
477480
return user_has_permission(
478481
request.user, engagement, Permissions.Import_Scan_Result,
479482
)
@@ -774,7 +777,13 @@ def has_permission(self, request, view):
774777
raise ValidationError(e)
775778

776779
if test := converted_dict.get("test"):
777-
# existing test, nothing special to check
780+
# Validate the resolved test's parent chain matches any provided names
781+
if (product_name := converted_dict.get("product_name")) and test.engagement.product.name != product_name:
782+
msg = f'The resolved test is associated with product "{test.engagement.product.name}", not with product "{product_name}"'
783+
raise ValidationError(msg)
784+
if (engagement_name := converted_dict.get("engagement_name")) and test.engagement.name != engagement_name:
785+
msg = f'The resolved test is associated with engagement "{test.engagement.name}", not with engagement "{engagement_name}"'
786+
raise ValidationError(msg)
778787
return user_has_permission(
779788
request.user, test, Permissions.Import_Scan_Result,
780789
)
@@ -1181,7 +1190,10 @@ def check_auto_create_permission(
11811190
raise ValidationError(msg)
11821191

11831192
if engagement:
1184-
# existing engagement, nothing special to check
1193+
# Validate the resolved engagement's parent chain matches any provided names
1194+
if product is not None and engagement.product_id != product.id:
1195+
msg = f'The resolved engagement is associated with product "{engagement.product.name}", not with product "{product_name}"'
1196+
raise ValidationError(msg)
11851197
return user_has_permission(
11861198
user, engagement, Permissions.Import_Scan_Result,
11871199
)

dojo/importers/auto_create_context.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,9 @@ def get_target_engagement_if_exists(
181181
"""
182182
if engagement := get_object_or_none(Engagement, pk=engagement_id):
183183
logger.debug("Using existing engagement by id: %s", engagement_id)
184+
if product is not None and engagement.product_id != product.id:
185+
msg = f"Engagement \"{engagement_id}\" does not belong to product \"{product}\""
186+
raise ValueError(msg)
184187
return engagement
185188
# if there's no product, then for sure there's no engagement either
186189
if product is None:
@@ -203,6 +206,9 @@ def get_target_test_if_exists(
203206
"""
204207
if test := get_object_or_none(Test, pk=test_id):
205208
logger.debug("Using existing Test by id: %s", test_id)
209+
if engagement is not None and test.engagement_id != engagement.id:
210+
msg = f"Test \"{test_id}\" does not belong to engagement \"{engagement}\""
211+
raise ValueError(msg)
206212
return test
207213
# If the engagement is not supplied, we cannot do anything
208214
if not engagement:

unittests/test_rest_framework.py

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3429,6 +3429,85 @@ def test_create_not_authorized_product_name_engagement_name_scan_type_title(self
34293429
importer_mock.assert_not_called()
34303430
reimporter_mock.assert_not_called()
34313431

3432+
# Security tests: verify that conflicting ID-based and name-based identifiers are rejected
3433+
3434+
@patch("dojo.importers.default_reimporter.DefaultReImporter.process_scan")
3435+
@patch("dojo.importers.default_importer.DefaultImporter.process_scan")
3436+
def test_reimport_with_engagement_id_mismatched_product_name_is_rejected(self, importer_mock, reimporter_mock):
3437+
"""Sending engagement ID from one product with product_name from another must be rejected."""
3438+
importer_mock.return_value = IMPORTER_MOCK_RETURN_VALUE
3439+
reimporter_mock.return_value = REIMPORTER_MOCK_RETURN_VALUE
3440+
3441+
with Path("tests/zap_sample.xml").open(encoding="utf-8") as testfile:
3442+
payload = {
3443+
"minimum_severity": "Low",
3444+
"active": True,
3445+
"verified": True,
3446+
"scan_type": "ZAP Scan",
3447+
"file": testfile,
3448+
# Engagement 1 belongs to Product 2 ("Security How-to")
3449+
"engagement": 1,
3450+
# But product_name points to Product 1 ("Python How-to")
3451+
"product_name": "Python How-to",
3452+
"engagement_name": "April monthly engagement",
3453+
"version": "1.0.0",
3454+
}
3455+
response = self.client.post(self.url, payload)
3456+
self.assertEqual(400, response.status_code, response.content[:1000])
3457+
importer_mock.assert_not_called()
3458+
reimporter_mock.assert_not_called()
3459+
3460+
@patch("dojo.importers.default_reimporter.DefaultReImporter.process_scan")
3461+
@patch("dojo.importers.default_importer.DefaultImporter.process_scan")
3462+
def test_reimport_with_test_id_mismatched_product_name_is_rejected(self, importer_mock, reimporter_mock):
3463+
"""Sending test ID from one product with product_name from another must be rejected."""
3464+
importer_mock.return_value = IMPORTER_MOCK_RETURN_VALUE
3465+
reimporter_mock.return_value = REIMPORTER_MOCK_RETURN_VALUE
3466+
3467+
with Path("tests/zap_sample.xml").open(encoding="utf-8") as testfile:
3468+
payload = {
3469+
"minimum_severity": "Low",
3470+
"active": True,
3471+
"verified": True,
3472+
"scan_type": "ZAP Scan",
3473+
"file": testfile,
3474+
# Test 3 belongs to Engagement 1 -> Product 2 ("Security How-to")
3475+
"test": 3,
3476+
# But product_name points to Product 1 ("Python How-to")
3477+
"product_name": "Python How-to",
3478+
"version": "1.0.0",
3479+
}
3480+
response = self.client.post(self.url, payload)
3481+
self.assertEqual(400, response.status_code, response.content[:1000])
3482+
importer_mock.assert_not_called()
3483+
reimporter_mock.assert_not_called()
3484+
3485+
@patch("dojo.importers.default_reimporter.DefaultReImporter.process_scan")
3486+
@patch("dojo.importers.default_importer.DefaultImporter.process_scan")
3487+
def test_reimport_with_test_id_mismatched_engagement_name_is_rejected(self, importer_mock, reimporter_mock):
3488+
"""Sending test ID from one engagement with engagement_name from another must be rejected."""
3489+
importer_mock.return_value = IMPORTER_MOCK_RETURN_VALUE
3490+
reimporter_mock.return_value = REIMPORTER_MOCK_RETURN_VALUE
3491+
3492+
with Path("tests/zap_sample.xml").open(encoding="utf-8") as testfile:
3493+
payload = {
3494+
"minimum_severity": "Low",
3495+
"active": True,
3496+
"verified": True,
3497+
"scan_type": "ZAP Scan",
3498+
"file": testfile,
3499+
# Test 3 belongs to Engagement 1 ("1st Quarter Engagement")
3500+
"test": 3,
3501+
# But engagement_name points to a different engagement
3502+
"product_name": "Security How-to",
3503+
"engagement_name": "April monthly engagement",
3504+
"version": "1.0.0",
3505+
}
3506+
response = self.client.post(self.url, payload)
3507+
self.assertEqual(400, response.status_code, response.content[:1000])
3508+
importer_mock.assert_not_called()
3509+
reimporter_mock.assert_not_called()
3510+
34323511

34333512
@versioned_fixtures
34343513
class ProductTypeTest(BaseClass.BaseClassTest):

0 commit comments

Comments
 (0)