Skip to content

Commit 25b910d

Browse files
devGregAclaude
andcommitted
Fix permission vulnerabilities: IDOR, data exposure, and decorator misconfigs
Security fixes across the authorization stack: - serializers.py: Gate accepted_risks behind Risk_Acceptance permission so Readers can no longer see risk acceptance details via the API - views.py (api_v2): Add defense-in-depth permission checks to metadata batch operations; add clarifying comments to generate_report actions - benchmark/views.py: Fix IDOR allowing cross-product benchmark updates by validating product ownership after object lookup - cred/views.py: Fix 14 decorator misconfigurations — standalone views now use @user_is_configuration_authorized, product-scoped views use correct parent model types (Engagement/Test/Finding instead of Product) - object/views.py: Harden edit/delete with get_object_or_404 and parent product mismatch checks raising PermissionDenied - tool_product/views.py: Same hardening as object/views.py New test file: unittests/test_permissions_audit.py with 11 tests covering risk acceptance exposure, metadata batch auth, note relationship checks, benchmark IDOR, and object/tool_product parent validation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent db8b6c6 commit 25b910d

7 files changed

Lines changed: 671 additions & 62 deletions

File tree

dojo/api_v2/serializers.py

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1760,9 +1760,7 @@ class FindingSerializer(serializers.ModelSerializer):
17601760
mitigated_by = serializers.PrimaryKeyRelatedField(required=False, allow_null=True, queryset=User.objects.all())
17611761
tags = TagListSerializerField(required=False)
17621762
request_response = serializers.SerializerMethodField()
1763-
accepted_risks = RiskAcceptanceSerializer(
1764-
many=True, read_only=True, source="risk_acceptance_set",
1765-
)
1763+
accepted_risks = serializers.SerializerMethodField()
17661764
push_to_jira = serializers.BooleanField(default=False)
17671765
found_by = serializers.PrimaryKeyRelatedField(
17681766
queryset=Test_Type.objects.all(), many=True,
@@ -1806,6 +1804,16 @@ def __init__(self, *args, **kwargs):
18061804
many=True, required=False, queryset=Endpoint.objects.all(),
18071805
)
18081806

1807+
def get_accepted_risks(self, obj):
1808+
request = self.context.get("request")
1809+
if request is None:
1810+
return []
1811+
if not user_has_permission(request.user, obj, Permissions.Risk_Acceptance):
1812+
return []
1813+
return RiskAcceptanceSerializer(
1814+
obj.risk_acceptance_set.all(), many=True,
1815+
).data
1816+
18091817
@extend_schema_field(serializers.DateTimeField())
18101818
def get_jira_creation(self, obj):
18111819
return jira_helper.get_jira_creation(obj)

dojo/api_v2/views.py

Lines changed: 62 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
serializers,
4646
)
4747
from dojo.api_v2.prefetch.prefetcher import _Prefetcher
48+
from dojo.authorization.authorization import user_has_permission_or_403
4849
from dojo.authorization.roles_permissions import Permissions
4950
from dojo.celery_dispatch import dojo_dispatch_task
5051
from dojo.cred.queries import get_authorized_cred_mappings
@@ -351,7 +352,11 @@ def get_queryset(self):
351352
responses={status.HTTP_200_OK: serializers.ReportGenerateSerializer},
352353
)
353354
@action(
354-
detail=True, methods=["post"], permission_classes=[IsAuthenticated],
355+
detail=True, methods=["post"],
356+
# IsAuthenticated only: report generation requires View permission,
357+
# enforced by the permission-filtered get_queryset(). The viewset's
358+
# permission_classes would check Edit (POST), which is too restrictive.
359+
permission_classes=[IsAuthenticated],
355360
)
356361
def generate_report(self, request, pk=None):
357362
endpoint = self.get_object()
@@ -475,7 +480,11 @@ def reopen(self, request, pk=None):
475480
responses={status.HTTP_200_OK: serializers.ReportGenerateSerializer},
476481
)
477482
@action(
478-
detail=True, methods=["post"], permission_classes=[IsAuthenticated],
483+
detail=True, methods=["post"],
484+
# IsAuthenticated only: report generation requires View permission,
485+
# enforced by the permission-filtered get_queryset(). The viewset's
486+
# permission_classes would check Edit (POST), which is too restrictive.
487+
permission_classes=[IsAuthenticated],
479488
)
480489
def generate_report(self, request, pk=None):
481490
engagement = self.get_object()
@@ -1383,7 +1392,11 @@ def set_finding_as_original(self, request, pk, new_fid):
13831392
responses={status.HTTP_200_OK: serializers.ReportGenerateSerializer},
13841393
)
13851394
@action(
1386-
detail=False, methods=["post"], permission_classes=[IsAuthenticated],
1395+
detail=False, methods=["post"],
1396+
# IsAuthenticated only: report generation requires View permission,
1397+
# enforced by the permission-filtered get_queryset(). The viewset's
1398+
# permission_classes would check Edit (POST), which is too restrictive.
1399+
permission_classes=[IsAuthenticated],
13871400
)
13881401
def generate_report(self, request):
13891402
findings = self.get_queryset()
@@ -1728,38 +1741,55 @@ def batch(self, request, pk=None):
17281741
serialized_data = serializers.MetaMainSerializer(data=request.data)
17291742
if serialized_data.is_valid(raise_exception=True):
17301743
if request.method == "POST":
1731-
self.process_post(request.data)
1744+
self.process_post(request)
17321745
status_code = status.HTTP_201_CREATED
17331746
if request.method == "PATCH":
1734-
self.process_patch(request.data)
1747+
self.process_patch(request)
17351748
status_code = status.HTTP_200_OK
17361749

17371750
return Response(status=status_code, data=serialized_data.data)
17381751

1739-
def process_post(self: object, data: dict):
1740-
product = Product.objects.filter(id=data.get("product")).first()
1741-
finding = Finding.objects.filter(id=data.get("finding")).first()
1742-
endpoint = Endpoint.objects.filter(id=data.get("endpoint")).first()
1752+
def _fetch_and_authorize_parents(self, request, permission_map):
1753+
"""Fetch parent objects and verify the user has the required permissions."""
1754+
data = request.data
1755+
parents = {}
1756+
for field, (model, permission) in permission_map.items():
1757+
obj = model.objects.filter(id=data.get(field)).first()
1758+
if obj:
1759+
user_has_permission_or_403(request.user, obj, permission)
1760+
parents[field] = obj
1761+
return parents
1762+
1763+
def process_post(self, request):
1764+
data = request.data
1765+
parents = self._fetch_and_authorize_parents(request, {
1766+
"product": (Product, Permissions.Product_Edit),
1767+
"finding": (Finding, Permissions.Finding_Edit),
1768+
"endpoint": (Endpoint, Permissions.Location_Edit),
1769+
})
17431770
metalist = data.get("metadata")
17441771
for metadata in metalist:
17451772
try:
17461773
DojoMeta.objects.create(
1747-
product=product,
1748-
finding=finding,
1749-
endpoint=endpoint,
1774+
product=parents["product"],
1775+
finding=parents["finding"],
1776+
endpoint=parents["endpoint"],
17501777
name=metadata.get("name"),
17511778
value=metadata.get("value"),
17521779
)
17531780
except (IntegrityError) as ex: # this should not happen as the data was validated in the batch call
17541781
raise ValidationError(str(ex))
17551782

1756-
def process_patch(self: object, data: dict):
1757-
product = Product.objects.filter(id=data.get("product")).first()
1758-
finding = Finding.objects.filter(id=data.get("finding")).first()
1759-
endpoint = Endpoint.objects.filter(id=data.get("endpoint")).first()
1783+
def process_patch(self, request):
1784+
data = request.data
1785+
parents = self._fetch_and_authorize_parents(request, {
1786+
"product": (Product, Permissions.Product_Edit),
1787+
"finding": (Finding, Permissions.Finding_Edit),
1788+
"endpoint": (Endpoint, Permissions.Location_Edit),
1789+
})
17601790
metalist = data.get("metadata")
17611791
for metadata in metalist:
1762-
dojometa = DojoMeta.objects.filter(product=product, finding=finding, endpoint=endpoint, name=metadata.get("name"))
1792+
dojometa = DojoMeta.objects.filter(product=parents["product"], finding=parents["finding"], endpoint=parents["endpoint"], name=metadata.get("name"))
17631793
if dojometa:
17641794
try:
17651795
dojometa.update(
@@ -1815,7 +1845,11 @@ def destroy(self, request, *args, **kwargs):
18151845
responses={status.HTTP_200_OK: serializers.ReportGenerateSerializer},
18161846
)
18171847
@action(
1818-
detail=True, methods=["post"], permission_classes=[IsAuthenticated],
1848+
detail=True, methods=["post"],
1849+
# IsAuthenticated only: report generation requires View permission,
1850+
# enforced by the permission-filtered get_queryset(). The viewset's
1851+
# permission_classes would check Edit (POST), which is too restrictive.
1852+
permission_classes=[IsAuthenticated],
18191853
)
18201854
def generate_report(self, request, pk=None):
18211855
product = self.get_object()
@@ -1956,7 +1990,11 @@ def destroy(self, request, *args, **kwargs):
19561990
responses={status.HTTP_200_OK: serializers.ReportGenerateSerializer},
19571991
)
19581992
@action(
1959-
detail=True, methods=["post"], permission_classes=[IsAuthenticated],
1993+
detail=True, methods=["post"],
1994+
# IsAuthenticated only: report generation requires View permission,
1995+
# enforced by the permission-filtered get_queryset(). The viewset's
1996+
# permission_classes would check Edit (POST), which is too restrictive.
1997+
permission_classes=[IsAuthenticated],
19601998
)
19611999
def generate_report(self, request, pk=None):
19622000
product_type = self.get_object()
@@ -2143,7 +2181,11 @@ def get_serializer_class(self):
21432181
responses={status.HTTP_200_OK: serializers.ReportGenerateSerializer},
21442182
)
21452183
@action(
2146-
detail=True, methods=["post"], permission_classes=[IsAuthenticated],
2184+
detail=True, methods=["post"],
2185+
# IsAuthenticated only: report generation requires View permission,
2186+
# enforced by the permission-filtered get_queryset(). The viewset's
2187+
# permission_classes would check Edit (POST), which is too restrictive.
2188+
permission_classes=[IsAuthenticated],
21472189
)
21482190
def generate_report(self, request, pk=None):
21492191
test = self.get_object()

dojo/benchmark/views.py

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ def update_benchmark(request, pid, _type):
4646
field = request.POST.get("field")
4747
value = request.POST.get("value")
4848
value = {"true": True, "false": False}.get(value, value)
49+
product = get_object_or_404(Product, id=pid)
50+
bench = get_object_or_404(Benchmark_Product.objects.filter(product=product), id=bench_id)
4951

5052
if field in {
5153
"enabled",
@@ -54,7 +56,6 @@ def update_benchmark(request, pid, _type):
5456
"get_notes",
5557
"delete_notes",
5658
}:
57-
bench = Benchmark_Product.objects.get(id=bench_id)
5859
if field == "enabled":
5960
bench.enabled = value
6061
elif field == "pass_fail":
@@ -90,21 +91,22 @@ def update_benchmark(request, pid, _type):
9091
@user_is_authorized(Product, Permissions.Benchmark_Edit, "pid")
9192
def update_benchmark_summary(request, pid, _type, summary):
9293
if request.method == "POST":
94+
product = get_object_or_404(Product, id=pid)
95+
benchmark_summary = get_object_or_404(Benchmark_Product_Summary.objects.filter(product=product), id=summary)
9396
field = request.POST.get("field")
9497
value = request.POST.get("value")
9598
value = {"true": True, "false": False}.get(value, value)
9699

97100
if field in {"publish", "desired_level"}:
98-
summary = Benchmark_Product_Summary.objects.get(id=summary)
99101
data = {}
100102
if field == "publish":
101-
summary.publish = value
103+
benchmark_summary.publish = value
102104
data = {"publish": value}
103105
elif field == "desired_level":
104-
summary.desired_level = value
105-
data = {"desired_level": value, "text": asvs_level(summary)}
106+
benchmark_summary.desired_level = value
107+
data = {"desired_level": value, "text": asvs_level(benchmark_summary)}
106108

107-
summary.save()
109+
benchmark_summary.save()
108110
return JsonResponse(data)
109111

110112
return redirect_to_return_url_or_else(

dojo/cred/views.py

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ def all_cred_product(request, pid):
4747
return render(request, "dojo/view_cred_prod.html", {"product_tab": product_tab, "creds": creds, "prod": prod})
4848

4949

50-
@user_is_authorized(Cred_User, Permissions.Credential_Edit, "ttid")
50+
@user_is_configuration_authorized(Permissions.Credential_Edit)
5151
def edit_cred(request, ttid):
5252
tool_config = Cred_User.objects.get(pk=ttid)
5353
if request.method == "POST":
@@ -79,7 +79,7 @@ def edit_cred(request, ttid):
7979
})
8080

8181

82-
@user_is_authorized(Cred_User, Permissions.Credential_View, "ttid")
82+
@user_is_configuration_authorized(Permissions.Credential_View)
8383
def view_cred_details(request, ttid):
8484
cred = Cred_User.objects.get(pk=ttid)
8585
notes = cred.notes.all()
@@ -127,7 +127,7 @@ def cred(request):
127127

128128

129129
@user_is_authorized(Product, Permissions.Product_View, "pid")
130-
@user_is_authorized(Cred_User, Permissions.Credential_View, "ttid")
130+
@user_is_authorized(Cred_Mapping, Permissions.Credential_View, "ttid")
131131
def view_cred_product(request, pid, ttid):
132132
cred = get_object_or_404(
133133
Cred_Mapping.objects.select_related("cred_id"), id=ttid)
@@ -182,8 +182,8 @@ def view_cred_product(request, pid, ttid):
182182
})
183183

184184

185-
@user_is_authorized(Product, Permissions.Engagement_View, "eid")
186-
@user_is_authorized(Cred_User, Permissions.Credential_View, "ttid")
185+
@user_is_authorized(Engagement, Permissions.Engagement_View, "eid")
186+
@user_is_authorized(Cred_Mapping, Permissions.Credential_View, "ttid")
187187
def view_cred_product_engagement(request, eid, ttid):
188188
cred = get_object_or_404(
189189
Cred_Mapping.objects.select_related("cred_id"), id=ttid)
@@ -231,8 +231,8 @@ def view_cred_product_engagement(request, eid, ttid):
231231
})
232232

233233

234-
@user_is_authorized(Product, Permissions.Test_View, "tid")
235-
@user_is_authorized(Cred_User, Permissions.Credential_View, "ttid")
234+
@user_is_authorized(Test, Permissions.Test_View, "tid")
235+
@user_is_authorized(Cred_Mapping, Permissions.Credential_View, "ttid")
236236
def view_cred_engagement_test(request, tid, ttid):
237237
cred = get_object_or_404(
238238
Cred_Mapping.objects.select_related("cred_id"), id=ttid)
@@ -282,8 +282,8 @@ def view_cred_engagement_test(request, tid, ttid):
282282
})
283283

284284

285-
@user_is_authorized(Product, Permissions.Finding_View, "fid")
286-
@user_is_authorized(Cred_User, Permissions.Credential_View, "ttid")
285+
@user_is_authorized(Finding, Permissions.Finding_View, "fid")
286+
@user_is_authorized(Cred_Mapping, Permissions.Credential_View, "ttid")
287287
def view_cred_finding(request, fid, ttid):
288288
cred = get_object_or_404(
289289
Cred_Mapping.objects.select_related("cred_id"), id=ttid)
@@ -334,7 +334,7 @@ def view_cred_finding(request, fid, ttid):
334334

335335

336336
@user_is_authorized(Product, Permissions.Product_Edit, "pid")
337-
@user_is_authorized(Cred_User, Permissions.Credential_Edit, "ttid")
337+
@user_is_authorized(Cred_Mapping, Permissions.Credential_Edit, "ttid")
338338
def edit_cred_product(request, pid, ttid):
339339
cred = get_object_or_404(
340340
Cred_Mapping.objects.select_related("cred_id"), id=ttid)
@@ -362,7 +362,7 @@ def edit_cred_product(request, pid, ttid):
362362

363363

364364
@user_is_authorized(Engagement, Permissions.Engagement_Edit, "eid")
365-
@user_is_authorized(Cred_User, Permissions.Credential_Edit, "ttid")
365+
@user_is_authorized(Cred_Mapping, Permissions.Credential_Edit, "ttid")
366366
def edit_cred_product_engagement(request, eid, ttid):
367367
cred = get_object_or_404(
368368
Cred_Mapping.objects.select_related("cred_id"), id=ttid)
@@ -582,7 +582,6 @@ def new_cred_finding(request, fid):
582582
})
583583

584584

585-
@user_is_authorized(Cred_User, Permissions.Credential_Delete, "ttid")
586585
def delete_cred_controller(request, destination_url, elem_id, ttid):
587586
cred = Cred_Mapping.objects.filter(pk=ttid).first()
588587
if request.method == "POST":
@@ -662,30 +661,30 @@ def delete_cred_controller(request, destination_url, elem_id, ttid):
662661
})
663662

664663

665-
@user_is_authorized(Cred_User, Permissions.Credential_Delete, "ttid")
664+
@user_is_configuration_authorized(Permissions.Credential_Delete)
666665
def delete_cred(request, ttid):
667666
return delete_cred_controller(request, "cred", 0, ttid=ttid)
668667

669668

670669
@user_is_authorized(Product, Permissions.Product_Edit, "pid")
671-
@user_is_authorized(Cred_User, Permissions.Credential_Delete, "ttid")
670+
@user_is_authorized(Cred_Mapping, Permissions.Credential_Delete, "ttid")
672671
def delete_cred_product(request, pid, ttid):
673672
return delete_cred_controller(request, "all_cred_product", pid, ttid)
674673

675674

676675
@user_is_authorized(Engagement, Permissions.Engagement_Edit, "eid")
677-
@user_is_authorized(Cred_User, Permissions.Credential_Delete, "ttid")
676+
@user_is_authorized(Cred_Mapping, Permissions.Credential_Delete, "ttid")
678677
def delete_cred_engagement(request, eid, ttid):
679678
return delete_cred_controller(request, "view_engagement", eid, ttid)
680679

681680

682681
@user_is_authorized(Test, Permissions.Test_Edit, "tid")
683-
@user_is_authorized(Cred_User, Permissions.Credential_Delete, "ttid")
682+
@user_is_authorized(Cred_Mapping, Permissions.Credential_Delete, "ttid")
684683
def delete_cred_test(request, tid, ttid):
685684
return delete_cred_controller(request, "view_test", tid, ttid)
686685

687686

688687
@user_is_authorized(Finding, Permissions.Finding_Edit, "fid")
689-
@user_is_authorized(Cred_User, Permissions.Credential_Delete, "ttid")
688+
@user_is_authorized(Cred_Mapping, Permissions.Credential_Delete, "ttid")
690689
def delete_cred_finding(request, fid, ttid):
691690
return delete_cred_controller(request, "view_finding", fid, ttid)

dojo/object/views.py

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import logging
22

33
from django.contrib import messages
4-
from django.core.exceptions import BadRequest
4+
from django.core.exceptions import PermissionDenied
55
from django.http import HttpResponseRedirect
66
from django.shortcuts import get_object_or_404, render
77
from django.urls import reverse
@@ -63,12 +63,10 @@ def view_objects(request, pid):
6363

6464
@user_is_authorized(Product, Permissions.Product_Tracking_Files_Edit, "pid")
6565
def edit_object(request, pid, ttid):
66-
object_prod = Objects_Product.objects.get(pk=ttid)
66+
object_prod = get_object_or_404(Objects_Product, pk=ttid)
6767
product = get_object_or_404(Product, id=pid)
6868
if object_prod.product != product:
69-
msg = labels.ASSET_TRACKED_FILES_ID_MISMATCH_ERROR_MESSAGE % {"asset_id": pid,
70-
"object_asset_id": object_prod.product.id}
71-
raise BadRequest(msg)
69+
raise PermissionDenied
7270

7371
if request.method == "POST":
7472
tform = ObjectSettingsForm(request.POST, instance=object_prod)
@@ -94,12 +92,10 @@ def edit_object(request, pid, ttid):
9492

9593
@user_is_authorized(Product, Permissions.Product_Tracking_Files_Delete, "pid")
9694
def delete_object(request, pid, ttid):
97-
object_prod = Objects_Product.objects.get(pk=ttid)
95+
object_prod = get_object_or_404(Objects_Product, pk=ttid)
9896
product = get_object_or_404(Product, id=pid)
9997
if object_prod.product != product:
100-
msg = labels.ASSET_TRACKED_FILES_ID_MISMATCH_ERROR_MESSAGE % {"asset_id": pid,
101-
"object_asset_id": object_prod.product.id}
102-
raise BadRequest(msg)
98+
raise PermissionDenied
10399

104100
if request.method == "POST":
105101
tform = ObjectSettingsForm(request.POST, instance=object_prod)

0 commit comments

Comments
 (0)