Global Finding Groups page

[LeoOMaia]

I developed a new tab so that people can better see the information of the Finding_Group that is created during the scan import. This is acording to #12684.
As a next step we plan to allow include an option to dynamically create and visualize groups by properties of Findings.

Bellow the image of Findings tab:

And the image of the reduced result with Finding_Group tab:

[dryrunsecurity]

🔴 Risk threshold exceeded.

This pull request contains sensitive edits to multiple files in the dojo project, including filters, views, and templates, with potential security concerns such as an authorization bypass in FindingGroupsFilter and a possible Denial of Service vulnerability in the ListFindingGroups view.

🔴 Configured Codepaths Edit in dojo/filters.py

Vulnerability	Configured Codepaths Edit
Description	Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml.

🔴 Configured Codepaths Edit in dojo/finding_group/urls.py

Vulnerability	Configured Codepaths Edit
Description	Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml.

🔴 Configured Codepaths Edit in dojo/finding_group/views.py

Vulnerability	Configured Codepaths Edit
Description	Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml.

🔴 Configured Codepaths Edit in dojo/templates/base.html

Vulnerability	Configured Codepaths Edit
Description	Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml.

🔴 Configured Codepaths Edit in dojo/templates/dojo/finding_groups_list.html

Vulnerability	Configured Codepaths Edit
Description	Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml.

🔴 Configured Codepaths Edit in dojo/templates/dojo/finding_groups_list_snippet.html

Vulnerability	Configured Codepaths Edit
Description	Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml.

🔴 Configured Codepaths Edit in dojo/filters.py

Vulnerability	Configured Codepaths Edit
Description	Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml.

🔴 Configured Codepaths Edit in dojo/finding_group/views.py

Vulnerability	Configured Codepaths Edit
Description	Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml.

🔴 Configured Codepaths Edit in dojo/filters.py

Vulnerability	Configured Codepaths Edit
Description	Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml.

🔴 Configured Codepaths Edit in dojo/finding_group/views.py

Vulnerability	Configured Codepaths Edit
Description	Sensitive edits detected for this file. Sensitive file paths and allowed authors can be configured in .dryrunsecurity.yaml.

Authorization Bypass in FindingGroupsFilter in dojo/filters.py

Vulnerability

Authorization Bypass in FindingGroupsFilter

Description

The FindingGroupsFilter class, specifically its set_related_object_fields method, is designed to filter engagements based on a pid (product ID). While the ListFindingGroups view correctly uses get_authorized_products to retrieve products the user is authorized to view, the pid parameter is passed directly from the request to the FindingGroupsFilter without explicit authorization checks on the pid itself. This means an attacker could potentially manipulate the pid in the request to filter for engagements within a product they are not authorized to access. Although the final queryset in get_finding_groups is filtered by products (which are authorized), the initial filtering of the engagement queryset within FindingGroupsFilter's set_related_object_fields method based on an unvalidated pid could lead to an information leak or an authorization bypass if this filter is used in other contexts or if the subsequent authorization check is somehow bypassed or misconfigured.

django-DefectDojo/dojo/filters.py

Lines 2016 to 2055 in 9314f9d

	self.form.fields["reviewers"].queryset = self.form.fields["reporter"].queryset
	class FindingGroupsFilter(FilterSet):
	name = CharFilter(lookup_expr="icontains", label="Name")
	severity = ChoiceFilter(
	choices=[
	("Low", "Low"),
	("Medium", "Medium"),
	("High", "High"),
	("Critical", "Critical"),
	],
	label="Min Severity",
	)
	engagement = ModelMultipleChoiceFilter(queryset=Engagement.objects.none(), label="Engagement")
	product = ModelMultipleChoiceFilter(queryset=Product.objects.none(), label="Product")
	class Meta:
	model = Finding
	fields = ["name", "severity", "engagement", "product"]
	def __init__(self, *args, **kwargs):
	self.user = kwargs.pop("user", None)
	self.pid = kwargs.pop("pid", None)
	super().__init__(*args, **kwargs)
	self.set_related_object_fields()
	def set_related_object_fields(self):
	if self.pid is not None:
	self.form.fields["engagement"].queryset = Engagement.objects.filter(product_id=self.pid)
	if "product" in self.form.fields:
	del self.form.fields["product"]
	else:
	self.form.fields["product"].queryset = get_authorized_products(Permissions.Product_View)
	self.form.fields["engagement"].queryset = get_authorized_engagements(Permissions.Engagement_View)
	class AcceptedFindingFilter(FindingFilter):
	risk_acceptance__created__date = DateRangeFilter(label="Acceptance Date")
	risk_acceptance__owner = ModelMultipleChoiceFilter(

Potential Denial of Service (DoS) in dojo/finding_group/views.py

Vulnerability

Potential Denial of Service (DoS)

Description

The ListFindingGroups view is susceptible to a Denial of Service attack. The page_size parameter is taken directly from user input via request.GET.get("page_size", 25) without any upper bound validation, allowing an attacker to request an extremely large number of items per page. Additionally, the engagement and product filters use request.GET.getlist() and are directly incorporated into __in queries, which can lead to performance degradation if a large number of IDs are provided. The combination of these unbounded inputs with complex Django ORM queries involving distinct(), annotate(), Count(), and Subquery() can result in excessive database load and application server resource consumption, leading to a Denial of Service.

django-DefectDojo/dojo/finding_group/views.py

Lines 213 to 329 in 9314f9d

	"Error pushing to JIRA",
	extra_tags="alert-danger")
	return HttpResponse(status=500)
	class ListFindingGroups(View):
	filter_name: str = "All"
	SEVERITY_ORDER = {
	"Critical": 4,
	"High": 3,
	"Medium": 2,
	"Low": 1,
	"Info": 0,
	}
	def get_template(self) -> str:
	return "dojo/finding_groups_list.html"
	def order_field(self, request: HttpRequest, group_findings_queryset: QuerySet[Finding_Group]) -> QuerySet[Finding_Group]:
	order_field_param: str | None = request.GET.get("o")
	if order_field_param:
	reverse_order = order_field_param.startswith("-")
	order_field_param = order_field_param[1:] if reverse_order else order_field_param
	if order_field_param in {"name", "creator", "findings_count", "sla_deadline"}:
	prefix = "-" if reverse_order else ""
	group_findings_queryset = group_findings_queryset.order_by(f"{prefix}{order_field_param}")
	return group_findings_queryset
	def filters(self, request: HttpRequest) -> tuple[str, str | None, list[str], list[str]]:
	name_filter: str = request.GET.get("name", "").lower()
	min_severity_filter: str | None = request.GET.get("severity")
	engagement_filter: list[str] = request.GET.getlist("engagement")
	product_filter: list[str] = request.GET.getlist("product")
	return name_filter, min_severity_filter, engagement_filter, product_filter
	def filter_check(self, request: HttpRequest) -> Q:
	name_filter, min_severity_filter, engagement_filter, product_filter = self.filters(request)
	q_objects = Q()
	if name_filter:
	q_objects &= Q(name__icontains=name_filter)
	if product_filter:
	q_objects &= Q(findings__test__engagement__product__id__in=product_filter)
	if engagement_filter:
	q_objects &= Q(findings__test__engagement__id__in=engagement_filter)
	if min_severity_filter:
	min_severity_order_value = self.SEVERITY_ORDER.get(min_severity_filter, -1)
	valid_severities_for_filter = [
	sev for sev, order in self.SEVERITY_ORDER.items() if order >= min_severity_order_value
	]
	q_objects &= Q(findings__severity__in=valid_severities_for_filter)
	return q_objects
	def get_findings(self, products: QuerySet[Product] | None) -> tuple[QuerySet[Finding], QuerySet[Finding]]:
	filters: dict = {}
	if products:
	filters["test__engagement__product__in"] = products
	user_findings_qs = Finding.objects.filter(**filters)
	return user_findings_qs, user_findings_qs.filter(active=True)
	def get_finding_groups(self, request: HttpRequest, products: QuerySet[Product] | None = None) -> QuerySet[Finding_Group]:
	finding_groups_queryset = Finding_Group.objects.all()
	if products is not None:
	user_findings, _ = self.get_findings(products)
	finding_groups_queryset = finding_groups_queryset.filter(findings__id__in=Subquery(user_findings.values("id"))).distinct()
	request_filters_q = self.filter_check(request)
	finding_groups_queryset = finding_groups_queryset.filter(request_filters_q).distinct()
	finding_groups_queryset = finding_groups_queryset.annotate(
	findings_count=Count("findings", distinct=True),
	sla_deadline=Min("findings__sla_expiration_date"),
	)
	return self.order_field(request, finding_groups_queryset)
	def paginate_queryset(self, queryset: QuerySet[Finding_Group], request: HttpRequest) -> Page:
	page_size = int(request.GET.get("page_size", 25))
	paginator = Paginator(queryset, page_size)
	page_number = request.GET.get("page")
	return paginator.get_page(page_number)
	def get(self, request: HttpRequest) -> HttpResponse:
	global_role = Global_Role.objects.filter(user=request.user).first()
	products = get_authorized_products(Permissions.Product_View)
	if request.user.is_superuser or (global_role and global_role.role):
	finding_groups = self.get_finding_groups(request)
	elif products.exists():
	finding_groups = self.get_finding_groups(request, products)
	else:
	finding_groups = Finding_Group.objects.none()
	paginated_finding_groups = self.paginate_queryset(finding_groups, request)
	context = {
	"filter_name": self.filter_name,
	"filtered": FindingGroupsFilter(request.GET),
	"finding_groups": paginated_finding_groups,
	}
	add_breadcrumb(title="Finding Group", top_level=not request.GET, request=request)
	return render(request, self.get_template(), context)
	class ListOpenFindingGroups(ListFindingGroups):
	filter_name: str = "Open"
	def get_finding_groups(self, request: HttpRequest, products: QuerySet[Product] | None = None) -> QuerySet[Finding_Group]:
	finding_groups_queryset = super().get_finding_groups(request, products)
	_, active_findings = self.get_findings(products)
	return finding_groups_queryset.filter(findings__id__in=Subquery(active_findings.values("id"))).distinct()
	class ListClosedFindingGroups(ListFindingGroups):
	filter_name: str = "Closed"
	def get_finding_groups(self, request: HttpRequest, products: QuerySet[Product] | None = None) -> QuerySet[Finding_Group]:
	finding_groups_queryset = super().get_finding_groups(request, products)
	_, active_findings = self.get_findings(products)
	return finding_groups_queryset.exclude(findings__id__in=Subquery(active_findings.values("id"))).distinct()

We've notified @mtesauro.

---

All finding details can be found in the DryRun Security Dashboard.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

Conflicts have been resolved. A maintainer will review the pull request shortly.

[valentijnscholten]

I've placed some comments. Let me also check if the rest of the team also thinks this is a useful addition.

[LeoOMaia]

Okay, but I think it's better to just leave the title "Finding Group" page. I made it similar to the "Finding" page with the All/Open/Closed options.

[LeoOMaia]

One thing I noticed while implementing the code, which I think is worth mentioning: when we perform two scan imports, each with one finding that would be grouped into the same Finding_Group, DefectDojo is creating two groups with the same name instead of just including the finding from the second scan in the first group created, doubling the space occupied in the database. Is it supposed to be like this?

[valentijnscholten]

Converted to draft while waiting for completion.

[valentijnscholten]

One thing I noticed while implementing the code, which I think is worth mentioning: when we perform two scan imports, each with one finding that would be grouped into the same Finding_Group, DefectDojo is creating two groups with the same name instead of just including the finding from the second scan in the first group created, doubling the space occupied in the database. Is it supposed to be like this?

Finding_Groups are currently bound to a single test. So if you have two separate imports and separate Tests, there will be two finding groups created.

[mtesauro]

Approved