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

remove cve field from Findings #9908

Closed
wants to merge 5 commits into from

Conversation

manuel-sommer
Copy link
Contributor

This PR removes cve field as all cves were migrated to unsaved_vulnerability_ids. See #9791 (review)

@github-actions github-actions bot added the New Migration Adding a new migration file. Take care when merging. label Apr 10, 2024
Copy link

dryrunsecurity bot commented Apr 10, 2024

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

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

Note

🔴 Risk threshold exceeded. Adding a reviewer if one is configured in .dryrunsecurity.yaml.

notification list: @mtesauro @grendel513

Tip

Get answers to your security questions. Add a comment in this PR starting with @DryRunSecurity. For example...

@dryrunsecurity What are common security issues with web application cookies?

Powered by DryRun Security

@manuel-sommer
Copy link
Contributor Author

I will not fix Ruff Linter here as this is already covered here: #9903

Copy link

dryrunsecurity bot commented Jul 25, 2024

DryRun Security Summary

The pull request primarily focuses on refactoring the handling of vulnerability IDs (CVEs) in the application's data models, including the removal of the automatic setting of the cve field and the cve field itself from the Finding and Finding_Template models, indicating a shift towards a more explicit and flexible approach to managing CVE information.

Expand for full summary

Summary:

The code changes in this pull request are primarily focused on refactoring the handling of vulnerability IDs (CVEs) in the application's data models. The key changes include the removal of the automatic setting of the cve field on the Finding and Finding_Template models, as well as the removal of the cve field itself from these models. These changes suggest a shift towards a more explicit and flexible approach to managing CVE information in the application.

From an application security perspective, the removal of the automatic cve field population could lead to better data quality, as users will now be responsible for explicitly managing the CVE associations. This change also allows for more flexibility in how CVEs are represented, such as allowing multiple CVEs per finding or leaving the CVE field empty if the vulnerability does not have a known CVE. However, it's important to ensure that any potential impact on existing processes or integrations that rely on the previous behavior is properly addressed.

Additionally, the removal of the cve field from the Finding and Finding_Template models, along with the synchronization of the vulnerability_ids property, suggests a move towards a more organized and maintainable approach to managing vulnerability information in the application's data models. This refactoring effort could help reduce complexity and potential attack surface in the long run.

Files Changed:

  1. dojo/importers/base_importer.py: The changes in this file remove the synchronization of the cve field with the unsaved_vulnerability_ids field, suggesting that this functionality was no longer necessary.
  2. dojo/db_migrations/0213_remove_finding_dojo_findin_cve_dccd4b_idx_and_more.py: This database migration script removes an index and two fields related to CVE from the Finding and Finding_template models, indicating a shift in the application's approach to managing vulnerability information.
  3. dojo/finding/helper.py: The changes in this file remove the automatic setting of the cve field on the Finding and Finding_Template models, requiring the user to explicitly manage the CVE associations.
  4. dojo/models.py: The changes in this file remove the cve field from the Finding model and update the vulnerability_ids property to synchronize the removed cve field with the unsaved_vulnerability_ids list, suggesting a refactoring of the underlying data model.

Code Analysis

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

Analyzer Findings
Configured Codepaths Analyzer 6 findings

Riskiness

🔴 Risk threshold exceeded.

We've notified @mtesauro, @grendel513.

View PR in the DryRun Dashboard.

@github-actions github-actions bot removed the New Migration Adding a new migration file. Take care when merging. label Jul 25, 2024
@github-actions github-actions bot added the New Migration Adding a new migration file. Take care when merging. label Jul 25, 2024
@manuel-sommer
Copy link
Contributor Author

@Maffooch, can you guide me here?
I don't know how to get rid of the cve field in ./dojo/finding/views.py

Copy link

@Maffooch
Copy link
Contributor

@manuel-sommer I am a little nervous for this change to be processed at the moment. I am closing for now, and we can revisit in the future

@Maffooch Maffooch closed this Nov 29, 2024
@mtesauro
Copy link
Contributor

mtesauro commented Nov 29, 2024

@Maffooch 👍
Good call - the cve field really isn't totally necessary but keeping it for now can't hurt. We got other things I'd rather clean up first...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Migration Adding a new migration file. Take care when merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants