Merge branch 'bugfix' into report-css-fix

Author: paulOsinski

dojo/api_v2/permissions.py

@@ -473,7 +473,13 @@ def has_permission(self, request, view):
             # Raise an explicit drf exception here
             raise ValidationError(e)
         if engagement := converted_dict.get("engagement"):
-            # existing engagement, nothing special to check
+            # Validate the resolved engagement's parent chain matches any provided identifiers
+            if (product := converted_dict.get("product")) and engagement.product_id != product.id:
+                msg = "The provided identifiers are inconsistent — the engagement does not belong to the specified product."
+                raise ValidationError(msg)
+            if (engagement_name := converted_dict.get("engagement_name")) and engagement.name != engagement_name:
+                msg = "The provided identifiers are inconsistent — the engagement name does not match the specified engagement."
+                raise ValidationError(msg)
             return user_has_permission(
                 request.user, engagement, Permissions.Import_Scan_Result,
             )
@@ -764,6 +770,11 @@ def has_permission(self, request, view):
         try:
             converted_dict = auto_create.convert_querydict_to_dict(request.data)
             auto_create.process_import_meta_data_from_dict(converted_dict)
+            # engagement is not a declared field on ReImportScanSerializer and will be
+            # stripped during validation — don't use it in the permission check either,
+            # so the permission check resolves targets the same way execution does
+            converted_dict.pop("engagement", None)
+            converted_dict.pop("engagement_id", None)
             # Get an existing product
             converted_dict["product_type"] = auto_create.get_target_product_type_if_exists(**converted_dict)
             converted_dict["product"] = auto_create.get_target_product_if_exists(**converted_dict)
@@ -774,7 +785,20 @@ def has_permission(self, request, view):
             raise ValidationError(e)
 
         if test := converted_dict.get("test"):
-            # existing test, nothing special to check
+            # Validate the resolved test's parent chain matches any provided identifiers
+            if (product := converted_dict.get("product")) and test.engagement.product_id != product.id:
+                msg = "The provided identifiers are inconsistent — the test does not belong to the specified product."
+                raise ValidationError(msg)
+            if (engagement := converted_dict.get("engagement")) and test.engagement_id != engagement.id:
+                msg = "The provided identifiers are inconsistent — the test does not belong to the specified engagement."
+                raise ValidationError(msg)
+            # Also validate by name when the objects were not resolved (e.g. names that match no existing record)
+            if not converted_dict.get("product") and (product_name := converted_dict.get("product_name")) and test.engagement.product.name != product_name:
+                msg = "The provided identifiers are inconsistent — the test does not belong to the specified product."
+                raise ValidationError(msg)
+            if not converted_dict.get("engagement") and (engagement_name := converted_dict.get("engagement_name")) and test.engagement.name != engagement_name:
+                msg = "The provided identifiers are inconsistent — the test does not belong to the specified engagement."
+                raise ValidationError(msg)
             return user_has_permission(
                 request.user, test, Permissions.Import_Scan_Result,
             )
@@ -1181,7 +1205,10 @@ def check_auto_create_permission(
         raise ValidationError(msg)
 
     if engagement:
-        # existing engagement, nothing special to check
+        # Validate the resolved engagement's parent chain matches any provided names
+        if product is not None and engagement.product_id != product.id:
+            msg = "The provided identifiers are inconsistent — the engagement does not belong to the specified product."
+            raise ValidationError(msg)
         return user_has_permission(
             user, engagement, Permissions.Import_Scan_Result,
         )

dojo/api_v2/serializers.py

@@ -1123,6 +1123,18 @@ def validate(self, data):
             if data.get("target_start") > data.get("target_end"):
                 msg = "Your target start date exceeds your target end date"
                 raise serializers.ValidationError(msg)
+        if (
+            self.instance is not None
+            and "product" in data
+            and data.get("product") != self.instance.product
+            and not user_has_permission(
+                self.context["request"].user,
+                data.get("product"),
+                Permissions.Engagement_Edit,
+            )
+        ):
+            msg = "You are not permitted to edit engagements in the destination product"
+            raise PermissionDenied(msg)
         return data
 
     def build_relational_field(self, field_name, relation_info):

dojo/engagement/views.py

@@ -286,6 +286,12 @@ def edit_engagement(request, eid):
         if form.is_valid():
             # first save engagement details
             new_status = form.cleaned_data.get("status")
+            if form.cleaned_data.get("product") != engagement.product:
+                user_has_permission_or_403(
+                    request.user,
+                    form.cleaned_data.get("product"),
+                    Permissions.Engagement_Edit,
+                )
             engagement.product = form.cleaned_data.get("product")
             engagement = form.save(commit=False)
             if (new_status in {"Cancelled", "Completed"}):
@@ -1442,9 +1448,10 @@ def view_edit_risk_acceptance(request, eid, raid, *, edit_mode=False):
                     "Since you are not the note's author, it was not deleted.",
                     extra_tags="alert-danger")
 
-        if "remove_finding" in request.POST:
+        if edit_mode and "remove_finding" in request.POST:
             finding = get_object_or_404(
-                Finding, pk=request.POST["remove_finding_id"])
+                risk_acceptance.accepted_findings,
+                pk=request.POST["remove_finding_id"])
 
             ra_helper.remove_finding_from_risk_acceptance(request.user, risk_acceptance, finding)
 

dojo/importers/auto_create_context.py

@@ -181,6 +181,9 @@ def get_target_engagement_if_exists(
         """
         if engagement := get_object_or_none(Engagement, pk=engagement_id):
             logger.debug("Using existing engagement by id: %s", engagement_id)
+            if product is not None and engagement.product_id != product.id:
+                msg = "The provided identifiers are inconsistent — the engagement does not belong to the specified product."
+                raise ValueError(msg)
             return engagement
         # if there's no product, then for sure there's no engagement either
         if product is None:
@@ -203,6 +206,9 @@ def get_target_test_if_exists(
         """
         if test := get_object_or_none(Test, pk=test_id):
             logger.debug("Using existing Test by id: %s", test_id)
+            if engagement is not None and test.engagement_id != engagement.id:
+                msg = "The provided identifiers are inconsistent — the test does not belong to the specified engagement."
+                raise ValueError(msg)
             return test
         # If the engagement is not supplied, we cannot do anything
         if not engagement:

dojo/risk_acceptance/api.py

@@ -1,18 +1,20 @@
 from abc import ABC, abstractmethod
 from typing import NamedTuple
 
+from django.core.exceptions import PermissionDenied
 from django.db.models import QuerySet
 from django.utils import timezone
 from drf_spectacular.utils import extend_schema
 from rest_framework import serializers, status
 from rest_framework.decorators import action
-from rest_framework.permissions import IsAdminUser
+from rest_framework.permissions import IsAuthenticated
 from rest_framework.response import Response
 
+from dojo.api_v2.permissions import UserHasRiskAcceptanceRelatedObjectPermission
 from dojo.api_v2.serializers import RiskAcceptanceSerializer
 from dojo.authorization.roles_permissions import Permissions
 from dojo.engagement.queries import get_authorized_engagements
-from dojo.models import Risk_Acceptance, User, Vulnerability_Id
+from dojo.models import Engagement, Risk_Acceptance, User, Vulnerability_Id
 
 AcceptedRisk = NamedTuple("AcceptedRisk", (("vulnerability_id", str), ("justification", str), ("accepted_by", str)))
 
@@ -40,10 +42,13 @@ def risk_application_model_class(self):
         request=AcceptedRiskSerializer(many=True),
         responses={status.HTTP_201_CREATED: RiskAcceptanceSerializer(many=True)},
     )
-    @action(methods=["post"], detail=True, permission_classes=[IsAdminUser], serializer_class=AcceptedRiskSerializer,
-            filter_backends=[], pagination_class=None)
+    @action(methods=["post"], detail=True, permission_classes=[IsAuthenticated, UserHasRiskAcceptanceRelatedObjectPermission],
+            serializer_class=AcceptedRiskSerializer, filter_backends=[], pagination_class=None)
     def accept_risks(self, request, pk=None):
         model = self.get_object()
+        product = model.product if isinstance(model, Engagement) else model.engagement.product
+        if not product.enable_full_risk_acceptance:
+            raise PermissionDenied
         serializer = AcceptedRiskSerializer(data=request.data, many=True)
         if serializer.is_valid():
             accepted_risks = serializer.save()
@@ -63,7 +68,7 @@ class AcceptedFindingsMixin(ABC):
         request=AcceptedRiskSerializer(many=True),
         responses={status.HTTP_201_CREATED: RiskAcceptanceSerializer(many=True)},
     )
-    @action(methods=["post"], detail=False, permission_classes=[IsAdminUser], serializer_class=AcceptedRiskSerializer)
+    @action(methods=["post"], detail=False, permission_classes=[IsAuthenticated], serializer_class=AcceptedRiskSerializer)
     def accept_risks(self, request):
         serializer = AcceptedRiskSerializer(data=request.data, many=True)
         if serializer.is_valid():
@@ -72,7 +77,9 @@ def accept_risks(self, request):
             return Response(data=serializer.errors, status=status.HTTP_400_BAD_REQUEST)
         owner = request.user
         accepted_result = []
-        for engagement in get_authorized_engagements(Permissions.Engagement_View):
+        for engagement in get_authorized_engagements(Permissions.Risk_Acceptance):
+            if not engagement.product.enable_full_risk_acceptance:
+                continue
             base_findings = engagement.unaccepted_open_findings
             accepted = _accept_risks(accepted_risks, base_findings, owner)
             engagement.accept_risks(accepted)

dojo/templates/dojo/snippets/endpoints.html

@@ -72,13 +72,13 @@ <h6>Mitigated Endpoints / Systems ({{ finding.mitigated_endpoint_count }})</h6>
                                             {% if V3_FEATURE_LOCATIONS %}
                                                 <td style="word-break: break-word">{{ endpoint.location }}{% if endpoint.is_broken %} <span data-toggle="tooltip" title="Endpoint is broken. Check documentation to look for fix process" >&#128681;</span>{% endif %}</td>
                                                 <td>{{ endpoint.status }}</td>
-                                                <td>{{ endpoint.auditor|safe }}</td>
+                                                <td>{{ endpoint.auditor }}</td>
                                                 <td>{{ endpoint.audit_time|date }}</td>
                                             {% else %}
-                                                {% comment %} TODO: Delete this after the move to Locations {% endcomment %}    
+                                                {% comment %} TODO: Delete this after the move to Locations {% endcomment %}
                                                 <td style="word-break: break-word">{{ endpoint }}{% if endpoint.endpoint.is_broken %} <span data-toggle="tooltip" title="Endpoint is broken. Check documentation to look for fix process" >&#128681;</span>{% endif %}</td>
                                                 <td>{{ endpoint.status }}</td>
-                                                <td>{{ endpoint.mitigated_by|safe }}</td>
+                                                <td>{{ endpoint.mitigated_by }}</td>
                                                 <td>{{ endpoint.mitigated_time|date }}</td>
                                             {% endif %}
                                         </tr>
@@ -256,7 +256,7 @@ <h4>Mitigated Endpoints / Systems ({{ finding.mitigated_endpoint_count }})
                                                 {% include "dojo/snippets/tags.html" with tags=endpoint.location.tags.all %}
                                             </td>
                                             <td>{{ endpoint.get_status_display }}</td>
-                                            <td>{{ endpoint.auditor|safe }}</td>
+                                            <td>{{ endpoint.auditor }}</td>
                                             <td>{{ endpoint.audit_time|date }}</td>
                                         {% else %}
                                             {% comment %} TODO: Delete this after the move to Locations {% endcomment %}
@@ -265,7 +265,7 @@ <h4>Mitigated Endpoints / Systems ({{ finding.mitigated_endpoint_count }})
                                                 {% include "dojo/snippets/tags.html" with tags=endpoint.endpoint.tags.all %}
                                             </td>
                                             <td>{{ endpoint.status }}</td>
-                                            <td>{{ endpoint.mitigated_by|safe }}</td>
+                                            <td>{{ endpoint.mitigated_by }}</td>
                                             <td>{{ endpoint.mitigated_time|date }}</td>
                                         {% endif %}
                                     </tr>

dojo/tools/govulncheck/parser.py

@@ -40,6 +40,22 @@ def get_location(data, node):
     def get_version(data, node):
         return data["Requires"]["Modules"][str(node)]["Version"]
 
+    @staticmethod
+    def get_fix_info(affected_ranges):
+        for r in affected_ranges:
+            for event in r.get("events", []):
+                if "fixed" in event:
+                    return True, event["fixed"]
+        return False, ""
+
+    @staticmethod
+    def get_introduced_version(affected_ranges):
+        for r in affected_ranges:
+            for event in r.get("events", []):
+                if "introduced" in event:
+                    return event["introduced"]
+        return ""
+
     def get_finding_trace_info(self, data, osv_id):
         # Browse the findings to look for matching OSV-id. If the OSV-id is matching, extract traces.
         trace_info_strs = []
@@ -202,8 +218,12 @@ def get_findings(self, scan_file, test):
                         else:
                             title = f"{osv_data['id']} - {affected_package['name']}"
 
-                        affected_version = self.get_affected_version(data, osv_data["id"])
+                        fix_available, fix_version = self.get_fix_info(affected_ranges)
 
+                        affected_version = (
+                            self.get_affected_version(data, osv_data["id"])
+                            or self.get_introduced_version(affected_ranges)
+                        )
                         severity = elem["osv"].get("severity", SEVERITY)
 
                         d = {
@@ -215,6 +235,8 @@ def get_findings(self, scan_file, test):
                             "description": description,
                             "impact": impact,
                             "references": references,
+                            "fix_available": fix_available,
+                            "fix_version": fix_version,
                             "file_path": path,
                             "url": db_specific_url,
                             "unique_id_from_tool": osv_id,

dojo/tools/risk_recon/api.py

@@ -1,6 +1,7 @@
-import requests
 from django.conf import settings
 
+from dojo.utils_ssrf import SSRFError, make_ssrf_safe_session, validate_url_for_ssrf
+
 
 class RiskReconAPI:
     def __init__(self, api_key, endpoint, data):
@@ -26,7 +27,14 @@ def __init__(self, api_key, endpoint, data):
             raise Exception(msg)
         if self.url.endswith("/"):
             self.url = endpoint[:-1]
-        self.session = requests.Session()
+
+        try:
+            validate_url_for_ssrf(self.url)
+        except SSRFError as exc:
+            msg = f"Invalid Risk Recon API url: {exc}"
+            raise Exception(msg) from exc
+
+        self.session = make_ssrf_safe_session()
         self.map_toes()
         self.get_findings()
 
@@ -54,7 +62,7 @@ def map_toes(self):
                     filters = comps.get(name)
                     self.toe_map[toe_id] = filters or self.data
         else:
-            msg = f"Unable to query Target of Evaluations due to {response.status_code} - {response.content}"
+            msg = f"Unable to query Target of Evaluations due to {response.status_code}"
             raise Exception(msg)   # TODO: when implementing ruff BLE001, please fix also TODO in unittests/test_risk_recon.py
 
     def filter_finding(self, finding):
@@ -86,5 +94,5 @@ def get_findings(self):
                     if not self.filter_finding(finding):
                         self.findings.append(finding)
             else:
-                msg = f"Unable to collect findings from toe: {toe} due to {response.status_code} - {response.content}"
+                msg = f"Unable to collect findings from toe: {toe} due to {response.status_code}"
                 raise Exception(msg)