Skip to content

Conversation

@valentijnscholten
Copy link
Member

To help with understanding deduplication, the hash_code was already shown when hovering over the ID in the view finding screen. This PR adds the unique_id_from_tool to help the user understand deduplication. Both are also shown when hovering over the status in the list of findings.

@dryrunsecurity
Copy link

DryRun Security

🟡 Please give this pull request extra attention during review.

This pull request introduces templates that render finding_display_status output with the Django '|safe' filter (e.g., '{{ finding|finding_display_status|safe }}') and inserts unescaped attributes into title strings, which disables automatic escaping and creates a high-risk XSS sink because there is no evidence of context-appropriate sanitization of data returned by that filter.

🟡 Potential Cross-Site Scripting in dojo/templates/dojo/findings_list_snippet.html
Vulnerability Potential Cross-Site Scripting
Description The templates render {{ findingfinding_display_statussafe }} which applies the Django 'safe' filter and therefore disables automatic HTML-escaping. I traced that this is a custom template filter (finding_display_status) used in multiple templates. Because the 'safe' filter forces the output to be treated as trusted HTML, any user-controllable content returned by the finding_display_status filter (or data it includes) will be injected into the page without escaping, creating an XSS sink. The PR also adds unescaped attributes in title strings that include finding.unique_id_from_tool and finding.hash_code, but the primary high-risk issue is the explicit use of 'safe' on the finding_display_status output with no visible sanitization step in the provided patch.

{{ finding|finding_display_status|safe }} {{ finding|import_history }}
</td>
{% if system_settings.enable_jira %}

🟡 Potential Cross-Site Scripting in dojo/templates/dojo/view_test.html
Vulnerability Potential Cross-Site Scripting
Description The template explicitly applies the Django 'safe' filter to the output of the finding_display_status filter: '{{ findingfinding_display_statussafe }}'. The 'safe' filter disables Django's auto-escaping and will render any HTML returned by finding_display_status without escaping. I could not find any evidence in the provided patch that finding_display_status performs proper, context-appropriate sanitization (e.g., using Bleach or otherwise guaranteeing the HTML is safe). Without a verified sanitization step or a guarantee that the data is entirely trusted and contains no user-supplied content, this is a confirmed XSS risk: user-controllable content can reach an HTML rendering sink with escaping disabled.

{{ finding|finding_display_status|safe }}&nbsp;{{ finding|import_history }}
</td>
{% if system_settings.enable_jira %}


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

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

@Maffooch Maffooch requested review from Jino-T and rossops October 20, 2025 14:27
@Maffooch Maffooch merged commit e172143 into DefectDojo:bugfix Oct 20, 2025
149 of 150 checks passed
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.

5 participants