Handling "requires login" in "fingerprint" and "lines" fields of Semgrep JSON Report (issue #11480)

[farsheedify]

Description

This PR addresses the handling of the "requires login" value in the fingerprint field of Semgrep reports #11480. The change ensures that when the "requires login" value is encountered, it is treated as if it is not set, allowing the deduplication process to fall back to legacy deduplication based on hash codes.

Additionally, a new JSON report for Semgrep has been added to the unittests folder, and a function has been implemented in the test suite to verify this change. While I have not run the unit tests yet, I have tested the parser locally within my DefectDojo environment using the provided test JSON report. The results were compared with the Demo instance of DefectDojo to confirm the resolution of the bug.

Test results

The test suite has been extended with a new function to cover the changes in this PR.
I have not run the unit tests yet, but I have manually tested the parser with the newly added JSON report in my local instance of DefectDojo.
Results from both my local environment and the Demo instance of DefectDojo are attached to compare the resolution of the bug. The sample JSON report is also attached.
fingerprint_test.json

[dryrunsecurity]

DryRun Security Summary

The pull request enhances the Semgrep security testing tool by improving the parser's reliability, handling of findings, and introducing a new configuration file to identify potential security vulnerabilities in the application's codebase.

Expand for full summary

Summary:

This pull request includes several changes related to the security testing and analysis of the application using the Semgrep tool. The changes focus on improving the reliability and robustness of the Semgrep parser, as well as introducing a new Semgrep configuration file that identifies potential security vulnerabilities in the codebase.

The key changes include:

1. A new test case for the SemgrepParser class that ensures the parser correctly handles a specific type of input where the unique_id_from_tool attribute is None. This helps improve the overall reliability of the parser.

2. Changes to the Semgrep parser in the parser.py file to handle "requires login" fingerprint values and code snippets more consistently, ensuring that these findings are not overlooked or treated differently than other findings.

3. The introduction of a new Semgrep configuration file, fingerprint_test.json, that identifies two potential security vulnerabilities in the application's codebase: a command injection vulnerability in Python code and a potential SQL injection vulnerability in Go code. The report provides detailed information about the findings, including the associated OWASP and CWE references, as well as recommended mitigations.

From an application security perspective, these changes are positive and demonstrate a commitment to improving the security of the application. The new test case and parser changes help ensure the reliability of the security analysis tools, while the introduction of the Semgrep configuration file helps identify and address potential vulnerabilities in the codebase.

Files Changed:

1. unittests/tools/test_semgrep_parser.py: This file introduces a new test case for the SemgrepParser class, ensuring that the parser correctly handles a specific type of input where the unique_id_from_tool attribute is None.

2. dojo/tools/semgrep/parser.py: This file includes changes to the Semgrep parser to handle "requires login" fingerprint values and code snippets more consistently, ensuring that these findings are not overlooked or treated differently than other findings.

3. unittests/scans/semgrep/fingerprint_test.json: This new file introduces a Semgrep configuration that identifies two potential security vulnerabilities in the application's codebase: a command injection vulnerability in Python code and a potential SQL injection vulnerability in Go code.

Code Analysis

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

View PR in the DryRun Dashboard.

[valentijnscholten]

Thanks for the PR!

I think the lines section can contain the same string that should be ignored. Could you add the same logic there?

Also looks like you might need to rebase onto bugfix as there is now a superfluous commit not related to this PR.

[farsheedify]

@valentijnscholten
Sure, I'll go ahead and apply this to the PR.

[farsheedify]

The "requires login" for the "lines" field is now handled, and as a result, the snippet becomes null, as demonstrated in these two images.

[valentijnscholten]

Thanks, let's wait for a maintainer to allow the tests to run in GitHub.