Skip to content

Conversation

@Maffooch
Copy link
Contributor

Introduce a base class for related object permissions to streamline permission checks for engagements, risk acceptances, and findings. Update existing permission classes to utilize the new structure, enhancing code maintainability and clarity.

@Maffooch Maffooch requested a review from mtesauro as a code owner January 23, 2026 17:11
@github-actions github-actions bot added the apiv2 label Jan 23, 2026
@dryrunsecurity
Copy link

dryrunsecurity bot commented Jan 23, 2026

DryRun Security

This pull request introduces several authorization issues: multiple permission classes in dojo/api_v2 (e.g., BaseDjangoModelPermission, BaseRelatedObjectPermission, UserHasRiskAcceptancePermission) implement fail-open or unconditional allow logic that lets unauthorized methods and list/create actions bypass checks, and several Questionnaire-related viewsets lack explicit IsAuthenticated and use unfiltered querysets, allowing any authenticated user to list sensitive questionnaire and survey data across the system. These flaws can lead to information disclosure and unauthorized creation or modification of resources (e.g., risk acceptances) via the API.

Broken Access Control: Fail-open logic in BaseDjangoModelPermission in dojo/api_v2/permissions.py
Vulnerability Broken Access Control: Fail-open logic in BaseDjangoModelPermission
Description The BaseDjangoModelPermission class implements a fail-open authorization logic in its _evaluate_permissions method. If the HTTP request method is not explicitly defined in the permission map (such as HEAD, OPTIONS, or any custom methods), the method returns True, granting access by default. Furthermore, the has_permission method filters the map to only include GET and POST for list-level checks, meaning all other methods like PUT, PATCH, and DELETE are automatically allowed at the list level. This design facilitates unauthorized access and information leaks (e.g., using HEAD to probe for the existence of objects without having view permissions) and creates a significant risk of security bypasses if list-level actions are added using those methods.

if request.method not in permissions:
return True
# Evaluate the permissions as usual
for method, permission in permissions.items():

Authorization Bypass for List Endpoints in BaseRelatedObjectPermission in dojo/api_v2/permissions.py
Vulnerability Authorization Bypass for List Endpoints in BaseRelatedObjectPermission
Description The BaseRelatedObjectPermission class implements has_permission by unconditionally returning True. In Django Rest Framework (DRF), the has_permission method is the only permission check performed for list (GET collection) and create (POST collection) actions, as has_object_permission is only called for detail views. While some ViewSets correctly filter their results in get_queryset, several ViewSets (specifically QuestionnaireQuestionViewSet, QuestionnaireAnswerViewSet, QuestionnaireGeneralSurveyViewSet, QuestionnaireEngagementSurveyViewSet, and QuestionnaireAnsweredSurveyViewSet) use subclasses of BaseRelatedObjectPermission but return Model.objects.all() in their get_queryset method. Consequently, any authenticated user can list all questionnaire questions, answers, and surveys across the entire system, regardless of their actual permissions on the associated products or engagements.

return True
def has_object_permission(self, request: Request, view, obj):
return check_object_permission(

Broken Access Control: Unconditional permission grant in has_permission in dojo/api_v2/permissions.py
Vulnerability Broken Access Control: Unconditional permission grant in has_permission
Description The UserHasRiskAcceptancePermission class returns True unconditionally in its has_permission method. In Django Rest Framework, has_permission is the primary check for non-detail actions like list and create (POST). While the class properly implements has_object_permission for detail actions using the Risk_Acceptance permission, the create action bypasses this entirely. The RiskAcceptanceSerializer only checks if the user has Finding_Edit permission on the findings being accepted. Since Risk_Acceptance and Finding_Edit are distinct permissions in DefectDojo's authorization model, this allows a user who is authorized to edit findings but NOT authorized to accept risks to formally create Risk Acceptance objects via the API. This is a clear authorization bypass of the formal risk acceptance mechanism.

return True
def has_object_permission(self, request, view, obj):
return check_object_permission(

Broken Access Control: Missing IsAuthenticated requirement in Questionnaire ViewSets in dojo/api_v2/views.py
Vulnerability Broken Access Control: Missing IsAuthenticated requirement in Questionnaire ViewSets
Description The QuestionnaireAnsweredSurveyViewSet (and other Questionnaire-related ViewSets in hunks 31-34) lacks the IsAuthenticated permission class in its permission_classes list. While it includes DjangoModelPermissions, which by default requires authentication, it fails to follow the project's security pattern of explicitly requiring IsAuthenticated. More critically, the combination of UserHasEngagementRelatedObjectPermission (which permits all list views) and the standard DjangoModelPermissions (which does not check for 'view' permissions on GET requests) allows any authenticated user to list all answered surveys. This is further exacerbated by an unfiltered get_queryset method that returns all Answered_Survey objects, leading to unauthorized information disclosure of sensitive survey data to any user authenticated on the platform, regardless of their permissions for the associated engagements.

permission_classes = (
permissions.UserHasEngagementRelatedObjectPermission,
DjangoModelPermissions,
)


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

…ermission, UserHasRegulationPermission, and UserHasSLAPermission; update views accordingly
@Maffooch Maffooch added this to the 2.54.3 milestone Jan 23, 2026
Copy link
Contributor

@mtesauro mtesauro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants