Importer Close Old Findings: Accommodate different dedupe algorithms

[Maffooch]

When using close old findings with the Importer (not reimporter) the mechanism for identifying findings to be closed was only looking at dashcodes. This may not work very well for tools that are using unique ID from tool. This PR adds support for all deduplication algorithms for close old findings. Fixes #11227 with credit to @valentijnscholten for finding the optimal solution

[sc-9998]

[dryrunsecurity]

DryRun Security Summary

The code changes enhance DefectDojo's security finding management by improving deduplication algorithms, handling of closed findings, scope considerations, and service-based filtering, while also addressing potential command injection vulnerabilities identified through Semgrep scans.

Expand for full summary

Summary:

The code changes in this pull request cover various improvements and enhancements to the security finding import and deduplication process in the DefectDojo application. The changes span multiple files, including the BaseImporter, DefaultImporter, and DefaultReImporter classes, as well as some unit tests.

The key security-related changes are:

1. Deduplication Algorithm Flexibility: The changes introduce more flexibility in the choice of deduplication algorithms, including options like "hash_code", "unique_id_from_tool", and "unique_id_from_tool_or_hash_code". This allows the application to handle findings more accurately, depending on the report format and the organization's security requirements.

2. Improved Handling of Closed Findings: The changes in the DefaultImporter class ensure that findings that are already mitigated (closed) in the report are properly handled and not unnecessarily processed. This enhances the reliability and efficiency of the finding import process.

3. Expanded Scope Considerations: The changes now accommodate for the product scope or the engagement scope when closing old findings, instead of just the engagement scope. This provides more flexibility in managing findings across different projects and products.

4. Service-based Filtering: The changes introduce the ability to filter old findings based on the "service" field, which can be useful in complex environments with multiple services or applications.

5. Semgrep Vulnerability Detection: The unit tests include examples of Semgrep scans that identify potential command injection vulnerabilities in the sample/brute.py file. The recommended mitigation is to use shlex.quote() to properly sanitize user-controlled data before passing it to the subprocess.run() function.

Overall, these changes enhance the reliability, flexibility, and security of the finding import and management processes in the DefectDojo application. The focus on improving the deduplication algorithms, handling of closed findings, and scope considerations demonstrates a strong commitment to maintaining the integrity and accuracy of the security data within the application.

Files Changed:

1. dojo/importers/base_importer.py: Changes related to the determine_deduplication_algorithm method, which is responsible for choosing the appropriate deduplication algorithm for the test being processed.
2. unittests/scans/semgrep/close_old_findings_report_line31.json: A Semgrep scan report that identifies a potential command injection vulnerability in the sample/brute.py file.
3. dojo/importers/default_importer.py: Changes to the close_old_findings function, improving the handling of findings during the import process.
4. dojo/importers/default_reimporter.py: Changes to the match_new_finding_to_existing_finding method, allowing for more flexible deduplication algorithms.
5. unittests/scans/semgrep/close_old_findings_report_second_run_line24.json: Another Semgrep scan report identifying a potential command injection vulnerability in the sample/brute.py file.
6. unittests/scans/semgrep/close_old_findings_report_third_run_different_unique_id.json: A third Semgrep scan report, also identifying a potential command injection vulnerability in the sample/brute.py file.
7. unittests/test_importers_closeold.py: A new test case to verify the behavior of the DefaultImporter when handling findings with unique IDs from the tool.

Code Analysis

We ran 9 analyzers against 7 files and 0 analyzers had findings. 9 analyzers had no findings.

View PR in the DryRun Dashboard.

[dryrunsecurity]

DryRun Security Summary

The pull request implements security and reliability improvements to DefectDojo's import and deduplication processes, including enhanced deduplication algorithms, better handling of mitigated findings, scope-based filtering, improved vulnerability reporting, and identification of command injection vulnerabilities.

Expand for full summary

Summary:

The code changes in this pull request focus on improving the security and reliability of the DefectDojo application's import and deduplication processes. The key changes include:

1. Deduplication Algorithm Enhancements: The code introduces more flexible deduplication algorithms, allowing the importer to use different methods (e.g., hash code, unique ID from tool, or a combination) to match new findings to existing ones. This helps ensure that the deduplication process accurately identifies and tracks security vulnerabilities over time.

2. Handling of Mitigated Findings: The changes ensure that findings that have already been marked as "mitigated" (closed) are not accidentally closed again during the import process. This maintains the integrity of the application's vulnerability data.

3. Scope and Service-based Filtering: The code now supports filtering old findings based on the product, engagement, or associated service. This allows security teams to focus on the most relevant findings and avoid closing issues that are not applicable to the current context.

4. Vulnerability Reporting Improvements: The changes include updates to the unit tests, which validate the correct handling of findings with the same product but different unique IDs from the scanning tool. This helps ensure that the import process accurately tracks and reports on security vulnerabilities.

5. Command Injection Vulnerability Identification: The code changes also include Semgrep scan reports that identify a potential command injection vulnerability in the sample/brute.py file. This highlights the importance of properly sanitizing user-controlled input before using it in subprocess calls to prevent remote code execution.

Overall, these changes demonstrate a strong focus on improving the security and reliability of the DefectDojo application, which is a critical tool for managing and tracking security vulnerabilities in web applications.

Files Changed:

1. dojo/importers/default_reimporter.py: This file contains changes to the DefaultReImporter class, including the removal of the determine_deduplication_algorithm method and updates to the match_new_finding_to_existing_finding method to handle different deduplication algorithms.

2. unittests/scans/semgrep/close_old_findings_report_Line31.json: This file contains a Semgrep scan report that identifies a potential command injection vulnerability in the sample/brute.py file.

3. dojo/importers/base_importer.py: This file includes changes to the BaseImporter class, which provides the foundation for other importer classes. The changes focus on the determine_deduplication_algorithm method and the handling of asynchronous processing.

4. dojo/importers/default_importer.py: This file includes changes to the close_old_findings function, which now supports different deduplication algorithms, excludes closed findings, and provides scope-based and service-based filtering.

5. unittests/test_importers_closeold.py: This file contains a new test case to verify the behavior of the importer when dealing with findings that have the same product but different unique IDs from the scanning tool.

6. unittests/scans/semgrep/close_old_findings_report_third_run_different_unique_id.json and unittests/scans/semgrep/close_old_findings_report_second_run_line24.json: These files contain additional Semgrep scan reports that identify the same command injection vulnerability in the sample/brute.py file.

Code Analysis

We ran 9 analyzers against 7 files and 0 analyzers had findings. 9 analyzers had no findings.

View PR in the DryRun Dashboard.

[valentijnscholten]

lgtm

[mtesauro]

Approved