Skip to content

Conversation

@Maffooch
Copy link
Contributor

@Maffooch Maffooch commented Dec 5, 2025

Implement a null check for the engagement object in the permission validation for Risk Acceptance to prevent potential errors when accessing its properties. This change enhances the robustness of the permission checking logic.

[sc-12218]

@Maffooch Maffooch requested a review from mtesauro as a code owner December 5, 2025 20:52
@Maffooch Maffooch added the affects_pro PRs that affect Pro and need a coordinated release/merge moment. label Dec 5, 2025
@dryrunsecurity
Copy link

dryrunsecurity bot commented Dec 5, 2025

DryRun Security

This pull request introduces a potential authorization bypass: Risk_Acceptance objects can exist without an associated Engagement, and when that happens the permission check falls back to global permissions, allowing users with global rights to access or modify risk acceptances that should be subject to product/engagement-level controls.

Authorization Bypass in Risk Acceptance in dojo/authorization/authorization.py
Vulnerability Authorization Bypass in Risk Acceptance
Description The Risk_Acceptance model's engagement property can return None if a Risk_Acceptance object is not associated with any Engagement. The API endpoint /api/v2/accepted_findings/accept_risks/ allows the creation of Risk_Acceptance objects. These objects are associated with Finding objects, but not directly with an Engagement at the time of creation. The AcceptedFindingsMixin iterates through authorized engagements and calls engagement.accept_risks(accepted), which adds the Risk_Acceptance to the engagement's ManyToMany field. However, if a Risk_Acceptance is created and then later disassociated from all engagements (or if the initial association fails for some reason), it would exist without an engagement. In such a scenario, the permission check in user_has_permission for Risk_Acceptance objects would fall back to user_has_global_permission. This means a user with global permissions could access a Risk_Acceptance object that is not tied to any specific product/engagement, bypassing the intended granular product-level permission check.

if obj.engagement is not None:
return user_has_permission(user, obj.engagement.product, permission)
return user_has_global_permission(user, permission)


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

@valentijnscholten valentijnscholten added this to the 2.53.1 milestone Dec 6, 2025
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

affects_pro PRs that affect Pro and need a coordinated release/merge moment.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants