Skip to content

Conversation

Logicmn
Copy link

@Logicmn Logicmn commented Sep 30, 2025

Description

Hello again! This PR adds a new parser for GitHub secrets detection uploads.

Test results

Added unit tests for new parser. Results:

test_file_path_and_line_assignment (unittests.tools.test_github_secrets_detection_report_parser.TestGithubSecretsDetectionReportParser.test_file_path_and_line_assignment)
Test file path and line number extraction ... ok
test_parse_file_invalid_format_raises (unittests.tools.test_github_secrets_detection_report_parser.TestGithubSecretsDetectionReportParser.test_parse_file_invalid_format_raises)
Non-list JSON should raise ... ok
test_parse_file_with_multiple_vulns_has_multiple_findings (unittests.tools.test_github_secrets_detection_report_parser.TestGithubSecretsDetectionReportParser.test_parse_file_with_multiple_vulns_has_multiple_findings)
Multiple entries produce corresponding findings ... ok
test_parse_file_with_no_vuln_has_no_findings (unittests.tools.test_github_secrets_detection_report_parser.TestGithubSecretsDetectionReportParser.test_parse_file_with_no_vuln_has_no_findings)
Empty list should yield no findings ... ok
test_parse_file_with_one_vuln_parsed_correctly (unittests.tools.test_github_secrets_detection_report_parser.TestGithubSecretsDetectionReportParser.test_parse_file_with_one_vuln_parsed_correctly)
Single secret alert entry parsed correctly ... ok
test_severity_assignment (unittests.tools.test_github_secrets_detection_report_parser.TestGithubSecretsDetectionReportParser.test_severity_assignment)
Test severity assignment logic ... ok
test_url_setting (unittests.tools.test_github_secrets_detection_report_parser.TestGithubSecretsDetectionReportParser.test_url_setting)
Test URL assignment from GitHub alert ... ok

----------------------------------------------------------------------
Ran 7 tests in 0.054s

Ref links:

Copy link

dryrunsecurity bot commented Sep 30, 2025

DryRun Security

This pull request introduces a markdown/HTML injection risk in GithubSecretsDetectionReportParser: it builds Finding.description by concatenating unsanitized fields from the input JSON (e.g., html_url, secret_type_display_name, resolution_comment), which could allow stored XSS or content injection in DefectDojo when malicious markup or javascript is provided. The vuln is located in dojo/tools/github_secrets_detection_report/parser.py (lines 112–115) and should be mitigated by sanitizing or escaping user-supplied values before rendering.

Markdown/HTML Injection in Finding Description in dojo/tools/github_secrets_detection_report/parser.py
Vulnerability Markdown/HTML Injection in Finding Description
Description The GithubSecretsDetectionReportParser constructs the description for a Finding object by directly incorporating values from the input JSON report, such as html_url, secret_type_display_name, and resolution_comment. These values are not sanitized before being concatenated into the description string. If an attacker provides a crafted JSON report containing malicious Markdown or HTML within these fields, it could lead to Stored Cross-Site Scripting (XSS) or content injection when the Finding.description is rendered in the DefectDojo UI. For example, a malicious html_url could be javascript:alert(document.domain).

description = "\n\n".join(desc_lines)
# Determine severity based on state and other factors
if state == "resolved":


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

@github-actions github-actions bot added settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR docs labels Sep 30, 2025
Copy link
Member

@valentijnscholten valentijnscholten left a comment

Choose a reason for hiding this comment

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

Thank you for the PR and the defensive coding techniques. I posted a question about the severity logic and hash_code config.
Can you also base your PR against bugfix as requested in the PR template?

@valentijnscholten valentijnscholten added this to the 2.51.0 milestone Sep 30, 2025
@Logicmn Logicmn changed the base branch from master to bugfix September 30, 2025 17:28
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

@mtesauro mtesauro requested a review from dogboat October 2, 2025 15:22
@valentijnscholten valentijnscholten merged commit 46f95fc into DefectDojo:bugfix Oct 2, 2025
85 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs parser settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR unittests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants