Match all unchanged findings on reimport

[dziewxc]

This is a proposed fix of #3958
Currently when reimporting, only the first matching vulnerability is added to the list of unchanged items. So if there are 2 or more vulnerabilities with the same hash, only the first one will be considered unchanged. Rest of them will get mitigated. With this change all the matching vulnerabilities will be considered unchanged.

I tested several scenarios:

1. Report with duplicated vulns A and B was added and then reimported without changes: no findings were mitigated
2. After first reimport, other one was sent with only vulnerability A in the report: no findings were affected
3. Then the report without any of those findings was sent with reimport: all the findings got mitigated
4. After that I sent again report with finding A and all the duplicates were reactivated

I think this is how it should work, but I may miss some edge case.
About the dynamic finding - I wrote it to update only from the last one, I'm not sure what is it updating exactly but I suspect it shouldn't be called 2 times, the last one is enough.

[dryrunsecurity]

DryRun Security Summary

The code change in the default_reimporter.py file improves the re-import process in DefectDojo by updating the process_matched_finding method to handle multiple matched findings and ensure accurate endpoint status updates for dynamic findings.

Expand for full summary

Summary:

The code change in the dojo/importers/default_reimporter.py file is related to the processing of findings during a re-import operation in the DefectDojo application. The key changes include:

1. Updating the process_matched_finding method to handle the case where there are multiple existing findings that match the new finding. Previously, it only handled the first matched finding, but now it iterates through all the matched findings.
2. After processing the matched findings, the code updates the endpoints on the "last existing finding" (i.e., the last matched finding) if the finding is a dynamic finding.

This change ensures that when multiple existing findings match a new finding, the endpoint status is updated for all the matched findings, not just the first one. This is an important change, as it helps to ensure that the re-import process accurately updates the existing findings in the system.

From an application security perspective, this change is relevant because it improves the accuracy and completeness of the re-import process. Ensuring that all matched findings are properly updated, including their associated endpoints, helps to maintain an accurate and up-to-date view of the application's security posture. Overall, this change appears to be a bug fix or an enhancement to the re-import functionality in the DefectDojo application, which is an important component of an application security management system.

Files Changed:

• dojo/importers/default_reimporter.py: The code change in this file is related to the processing of findings during a re-import operation in the DefectDojo application. The key changes include updating the process_matched_finding method to handle the case where there are multiple existing findings that match the new finding, and updating the endpoints on the "last existing finding" (i.e., the last matched finding) if the finding is a dynamic finding.

Code Analysis

We ran 9 analyzers against 1 file and 1 analyzer had findings. 8 analyzers had no findings.

Analyzer	Findings
Configured Codepaths Analyzer	2 findings

Overall Riskiness

🔴 Risk threshold exceeded.

We've notified @mtesauro, @grendel513.

View PR in the DryRun Dashboard.

[valentijnscholten]

Thank you for the PR. One complication of this approach is that the endpoints are no longer updated for the first finding. This makes sense as we don't know which endpoints belong to which finding. Would there be something smart we can do here? Like maybe look at the endpoints (or other field(s)) to see if we can deduce an exact match?

Unsure about if we would need to inform the user of not being able to process the endpoints. We also need to find out what the maintainers think of this proposed adjustment in behaciour.

[dziewxc]

Thank you for the PR. One complication of this approach is that the endpoints are no longer updated for the first finding. This makes sense as we don't know which endpoints belong to which finding. Would there be something smart we can do here? Like maybe look at the endpoints (or other field(s)) to see if we can deduce an exact match?

Not sure to be honest. I assumed it was random before (the first finding was taken), so now I just changed it to the last one. But I don't use endpoints, so it's not obvious for me what is the expected behaviour here.

[Maffooch]

The endpoints are what make this exceptionally tricky. This problem will likely require a very involved and thought out solution. Closing this one for now

[dziewxc]

The endpoints are what make this exceptionally tricky. This problem will likely require a very involved and thought out solution. Closing this one for now

Do you think that someone would review the PR if I would figure out the way to handle the endpoints? We want to use the tool in our company, but Trivy sends us a lot of duplicates in reports. It makes tracking those vulnerabilities impossible because they got closed on second reimport.