Skip to content

Commit d079546

Browse files
author
Your Name
committed
Secure vulnerability analysis and fix data mapping race condition
1 parent 2c13ec9 commit d079546

4 files changed

Lines changed: 67 additions & 53 deletions

File tree

product_portfolio/models.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,7 @@ def can_be_changed_by(self, user):
364364

365365
return all(
366366
[
367+
not self.is_locked,
367368
user.has_perm("product_portfolio.change_product"),
368369
has_change_permission_on_product,
369370
]

product_portfolio/templates/product_portfolio/tabs/tab_packages_vulnerabilities.html

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
<td rowspan="{{ product_package.package.vulnerability_count }}">
3939
{% include 'vulnerabilities/includes/risk_score_badge.html' with risk_score=product_package.weighted_risk_score only %}
4040
</td>
41-
{% for vulnerability in product_package.package.affected_by_vulnerabilities.all %}
41+
{% for vulnerability in product_package.vulnerabilities_with_analysis %}
4242
{% if not forloop.first %}<tr>{% endif %}
4343
<td rowspan="{{ vulnerability.affected_packages_count }}">
4444
<strong>
@@ -100,16 +100,18 @@
100100
{% endif %}
101101
</td>
102102
<td class="p-1">
103-
<span data-bs-toggle="modal"
104-
data-bs-target="#vulnerability-analysis-modal"
105-
data-vulnerability-id="{{ vulnerability.vulnerability_id }}"
106-
data-package-identifier="{{ product_package.package.identifier }}"
107-
data-edit-url="{% url 'product_portfolio:vulnerability_analysis_form' product_package.uuid vulnerability.vulnerability_id %}"
108-
>
109-
<button type="button" data-bs-toggle="tooltip" title="Edit" class="btn btn-link p-0" aria-label="Edit">
110-
<i class="far fa-edit fa-sm"></i>
111-
</button>
112-
</span>
103+
{% if not product_package.product.is_locked %}
104+
<span data-bs-toggle="modal"
105+
data-bs-target="#vulnerability-analysis-modal"
106+
data-vulnerability-id="{{ vulnerability.vulnerability_id }}"
107+
data-package-identifier="{{ product_package.package.identifier }}"
108+
data-edit-url="{% url 'product_portfolio:vulnerability_analysis_form' product_package.uuid vulnerability.vulnerability_id %}"
109+
>
110+
<button type="button" data-bs-toggle="tooltip" title="Edit" class="btn btn-link p-0" aria-label="Edit">
111+
<i class="far fa-edit fa-sm"></i>
112+
</button>
113+
</span>
114+
{% endif %}
113115
</td>
114116
{% if not forloop.first %}</tr>{% endif %}
115117
{% endfor %}

product_portfolio/views.py

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
# See https://aboutcode.org for more information about AboutCode FOSS projects.
77
#
88

9+
import copy
910
import csv
1011
import json
1112
from collections import OrderedDict
@@ -34,6 +35,7 @@
3435
from django.http import FileResponse
3536
from django.http import Http404
3637
from django.http import HttpResponse
38+
from django.http import HttpResponseForbidden
3739
from django.http import JsonResponse
3840
from django.shortcuts import get_object_or_404
3941
from django.shortcuts import redirect
@@ -1236,13 +1238,18 @@ def get_context_data(self, **kwargs):
12361238
page_number = self.request.GET.get(self.query_dict_page_param)
12371239
page_obj = paginator.get_page(page_number)
12381240

1239-
# Set the proper VulnerabilityAnalysis instance on the Package instance
1241+
# Set the proper VulnerabilityAnalysis instance on each Vulnerability
12401242
for product_package in page_obj.object_list:
1243+
vulnerabilities_with_analysis = []
12411244
for vulnerability in product_package.package.affected_by_vulnerabilities.all():
1245+
v_copy = copy.copy(vulnerability)
1246+
v_copy.vulnerability_analysis = None
12421247
for analysis in vulnerability.vulnerability_analyses.all():
12431248
if analysis.product_package_id == product_package.id:
1244-
vulnerability.vulnerability_analysis = analysis
1245-
continue
1249+
v_copy.vulnerability_analysis = analysis
1250+
break
1251+
vulnerabilities_with_analysis.append(v_copy)
1252+
product_package.vulnerabilities_with_analysis = vulnerabilities_with_analysis
12461253

12471254
context_data.update(
12481255
{
@@ -2573,6 +2580,9 @@ def vulnerability_analysis_form_view(request, productpackage_uuid, vulnerability
25732580
product_package = get_object_or_404(product_package_qs, uuid=productpackage_uuid)
25742581
vulnerability = get_object_or_404(vulnerability_qs, vulnerability_id=vulnerability_id)
25752582

2583+
if not product_package.product.can_be_changed_by(user):
2584+
return HttpResponseForbidden("Permission denied: Product is locked")
2585+
25762586
# Fetch the existing Analysis values for each affected products
25772587
product_analysis = vulnerability_analysis_qs.filter(
25782588
product=OuterRef("pk"),

vulnerabilities/models.py

Lines changed: 40 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -570,46 +570,47 @@ def save(self, *args, **kwargs):
570570
def propagate(self, product_uuid, user):
571571
"""Propagate this Analysis to another Product."""
572572
from product_portfolio.models import ProductPackage
573-
574-
# Get the equivalent ProductPackage in the target product.
575573
product_package_qs = ProductPackage.objects.product_secured(user, perms="change_product")
576-
try:
577-
product_package = product_package_qs.get(
578-
product__uuid=product_uuid,
579-
package=self.package,
580-
dataspace=self.dataspace,
581-
)
582-
except models.ObjectDoesNotExist:
583-
return
584574

585-
target_analysis_base_data = {
586-
"product_package": product_package,
587-
"vulnerability": self.vulnerability,
588-
"dataspace": self.dataspace,
589-
}
590-
591-
existing_analysis = VulnerabilityAnalysis.objects.filter(**target_analysis_base_data)
592-
if existing_analysis: # Update
593-
target_analysis = existing_analysis[0]
594-
target_analysis.last_modified_by = user
595-
else: # New
596-
target_analysis = VulnerabilityAnalysis(
597-
**target_analysis_base_data,
598-
created_by=user,
599-
last_modified_by=user,
600-
)
575+
# Get all equivalent ProductPackage instances in the target product.
576+
target_product_packages = product_package_qs.filter(
577+
product__uuid=product_uuid,
578+
package=self.package,
579+
dataspace=self.dataspace,
580+
)
601581

602-
fields_to_clone = [
603-
"state",
604-
"justification",
605-
"responses",
606-
"detail",
607-
"is_reachable",
608-
]
609-
for field_name in fields_to_clone:
610-
field_value = getattr(self, field_name, None)
611-
if field_value is not None:
612-
setattr(target_analysis, field_name, field_value)
582+
propagated_analyses = []
583+
for product_package in target_product_packages:
584+
target_analysis_base_data = {
585+
"product_package": product_package,
586+
"vulnerability": self.vulnerability,
587+
"dataspace": self.dataspace,
588+
}
589+
590+
existing_analysis = VulnerabilityAnalysis.objects.filter(**target_analysis_base_data)
591+
if existing_analysis: # Update
592+
target_analysis = existing_analysis[0]
593+
target_analysis.last_modified_by = user
594+
else: # New
595+
target_analysis = VulnerabilityAnalysis(
596+
**target_analysis_base_data,
597+
created_by=user,
598+
last_modified_by=user,
599+
)
600+
601+
fields_to_clone = [
602+
"state",
603+
"justification",
604+
"responses",
605+
"detail",
606+
"is_reachable",
607+
]
608+
for field_name in fields_to_clone:
609+
field_value = getattr(self, field_name, None)
610+
if field_value is not None:
611+
setattr(target_analysis, field_name, field_value)
612+
613+
target_analysis.save()
614+
propagated_analyses.append(target_analysis)
613615

614-
target_analysis.save()
615-
return target_analysis
616+
return propagated_analyses

0 commit comments

Comments
 (0)