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", + )