Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 reset description in progpilot after each finding #10210

Merged
merged 2 commits into from
May 20, 2024

Conversation

manuel-sommer
Copy link
Contributor

see discussion in #10044

Copy link

dryrunsecurity bot commented May 15, 2024

Hi there 👋, @DryRunSecurity here, below is a summary of our analysis and findings.

DryRun Security Status Findings
Configured Codepaths Analyzer 0 findings
Sensitive Files Analyzer 0 findings
AppSec Analyzer 0 findings
Authn/Authz Analyzer 0 findings
Secrets Analyzer 0 findings

Note

🟢 Risk threshold not exceeded.

Change Summary (click to expand)

The following is a summary of changes in this pull request made by me, your security buddy 🤖.
Note that this summary is auto-generated and not meant to be a definitive list of security issues
but rather a helpful summary from a security perspective.

Summary:

The code change in this pull request is related to the ProgpilotParser class in the dojo/tools/progpilot/parser.py file. The change involves modifying the get_findings method to ensure that the description variable is reset for each finding during the iteration over the results list. This is a good practice to prevent any potential issues with findings sharing the same description, which could lead to data integrity or consistency problems.

From an application security perspective, the code appears to be handling the parsing of Progpilot security scan results correctly. The Finding objects created contain relevant information such as the vulnerability type, source and sink details, and the vulnerability description. The code also handles mapping the Progpilot-specific fields (e.g., vuln_cwe) to the appropriate fields in the Finding object, which is important for maintaining consistent data representation and analysis within the Dojo application. Overall, the code change seems to be a minor refactoring to improve the reliability and maintainability of the Progpilot parser.

Files Changed:

  • dojo/tools/progpilot/parser.py: The get_findings method in the ProgpilotParser class has been modified to move the description variable initialization inside the for loop that iterates over the results list. This ensures that the description variable is reset for each finding, preventing any potential issues with findings sharing the same description.

Powered by DryRun Security

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

@manuel-sommer
Copy link
Contributor Author

ah, lol. Thank you for the commit @cneill, you are right.

@nAgga3
Copy link

nAgga3 commented May 17, 2024

Would it be possible to approve the bug fix?

@Maffooch Maffooch merged commit cc8b8d8 into DefectDojo:bugfix May 20, 2024
123 checks passed
@nAgga3
Copy link

nAgga3 commented May 20, 2024

Good morning, @manuel-sommer unfortunately the fix does not resolve the parser issue. Unfortunately, the same behavior continues to occur that when there is a vulnerability of the "Security Misconfiguration" type, information from the previous vulnerability is brought and that when there are multiple vulnerabilities of the "Security Misconfiguration" type, it only brings information from a single one.

Sorry for my insistence

@manuel-sommer manuel-sommer deleted the progpilot_bug_descissue branch May 20, 2024 17:04
@manuel-sommer
Copy link
Contributor Author

manuel-sommer commented May 21, 2024

Hi @nAgga3 , did you test the latest version (2.34.4)?
I checked the code and did not see any inconsistencies. Please be more specific, e.g. make screenshots or upload a sample file. To me, this is fixed. You can open up a new issue if you want.

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.

6 participants