From 6e133974159250f971da55b3514c709c9b8e9be4 Mon Sep 17 00:00:00 2001 From: Sam Vader Date: Fri, 24 Jul 2026 18:40:39 -0500 Subject: [PATCH] Restrict bulk update target finding group to authorized groups The bulk finding update view looked up the target finding group without any authorization check, while the submitted findings were already scoped to the user's authorized products. Scope the group lookup the same way and add a regression test covering the cross-product case. --- dojo/finding/ui/views.py | 9 ++++-- unittests/test_bulk_finding_authorization.py | 32 ++++++++++++++++++-- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/dojo/finding/ui/views.py b/dojo/finding/ui/views.py index 9b0241421ff..24cfe1641ea 100644 --- a/dojo/finding/ui/views.py +++ b/dojo/finding/ui/views.py @@ -63,6 +63,7 @@ MergeFindings, ReviewFindingForm, ) +from dojo.finding_group.queries import get_authorized_finding_groups from dojo.forms import ( GITHUBFindingForm, JIRAFindingForm, @@ -81,7 +82,6 @@ Engagement, FileAccessToken, Finding, - Finding_Group, Finding_Template, GITHUB_Issue, GITHUB_PKey, @@ -2707,7 +2707,12 @@ def _bulk_update_finding_groups(finds, form): if form.cleaned_data["finding_group_add"]: logger.debug("finding_group_add checked!") fgid = form.cleaned_data["add_to_finding_group_id"] - finding_group = Finding_Group.objects.get(id=fgid) + # Scope the target group to the ones the user may edit, the same way the + # submitted findings are scoped above. Without this a caller could pass a + # group id from a product they have no access to. + finding_group = get_object_or_404( + get_authorized_finding_groups("edit"), id=fgid, + ) finding_group, added, skipped = finding_helper.add_to_finding_group( finding_group, finds, ) diff --git a/unittests/test_bulk_finding_authorization.py b/unittests/test_bulk_finding_authorization.py index de076f6f487..2dcd3e70a57 100644 --- a/unittests/test_bulk_finding_authorization.py +++ b/unittests/test_bulk_finding_authorization.py @@ -4,12 +4,13 @@ A product-scoped, non-staff user must not be able to bulk-delete (or edit) findings belonging to products they are not authorized for via ``finding_bulk_update_all`` (``/finding/bulk``), even by POSTing arbitrary -finding ids. +finding ids. The same scoping applies to the target finding group when adding +findings to a group. """ from django.urls import reverse -from dojo.models import Dojo_User, Finding, Test +from dojo.models import Dojo_User, Finding, Finding_Group, Test from .dojo_test_case import DojoTestCase, versioned_fixtures @@ -56,3 +57,30 @@ def test_scoped_user_cannot_bulk_edit_other_products_findings(self): self.other_finding.severity, original_severity, msg="scoped user edited a finding outside their authorized products", ) + + def test_scoped_user_cannot_add_finding_to_other_products_group(self): + # A finding the user is allowed to edit, in their authorized product, + # not yet part of any group. + my_finding = Finding.objects.filter( + test__engagement__product=self.product, + finding_group__isnull=True, + ).first() + self.assertIsNotNone(my_finding) + # A group that belongs to a different product. + other_test = Test.objects.exclude( + engagement__product=self.product, + ).first() + other_group = Finding_Group.objects.create( + name="scoping_regression_group", test=other_test, creator=self.user, + ) + response = self.client.post(reverse("finding_bulk_update_all"), { + "finding_to_update": [my_finding.id], + "finding_group_add": "true", + "add_to_finding_group_id": other_group.id, + }) + self.assertLess(response.status_code, 500) + self.assertNotIn( + my_finding.id, + list(other_group.findings.values_list("id", flat=True)), + msg="scoped user added a finding to a group outside their authorized products", + )