Improve cvssv3 validation

[valentijnscholten]

Although it's not fully clear "what the problem is" reported in #8264 , we want to stop using the regex for validation and use the cvss library in a validator. This PR updates the validator on the model and updates the test cases to reflect the new behaviour. The new behaviour is better:

• When saving via API or UI invalid vectors are recjted. (2 vectors, v3 vectors without a prefix, v3 vectors with a trailing slash, v4 vectors, ...)
• When Finding.save() is called, no validation is perfrmed by Django. So we have make sure the parsers only store valid vectors.

I updated the parsers that did not yet follow the guidelines in docs/content/en/open_source/contributing/how-to-write-a-parser.md. I considered:

• adding a set_data_from_cvss_string(...) to the Finding model
• adding code to Finding.save() to validate/parse the vector string

But in the end I went for a helper method in dojo.utils to parse the CVSS vector. This way we have the parsing logic in one central place and the parsers can decide which of the 3 parsed fields they want to use on the model (vector, score, severity). The logic is also failsafe, invalid vectors result in empty cvss fields but won't cause the import to fail.

Remaining questions:

• Do we want to keep the logic in place in Finding.save() that sets (overwrites) the cvssv3_score if a valid vector is set? (I think for now YES)
• What to we do when Finding.save() encounters an invalid vector in cvssv3. Should we clear the vector before saving? (I think for now NO)
• Do we want to make sure all parsers use the same/new parse function? (I think for now NO as long as they parse the vector correctly)

Risks:

• The parsers are fail-safe, but users submitting invalid CVSSv3 vectors through the API may run into HTTP_400 validation errors. But I'm not sure if that is a bad thing?

Feedback welcome. I also think this is a nice preparation for when we want to support CVSS4 vectors.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

Conflicts have been resolved. A maintainer will review the pull request shortly.

[dryrunsecurity]

This pull request introduces potential security risks related to CVSS vector parsing, logging, and input validation across multiple files, with concerns about incomplete data sanitization, reduced logging visibility, and potential exposure of sensitive vulnerability information.

💭 Unconfirmed Findings (9)

Vulnerability	Potential Information Disclosure via Logging
Description	In dojo/models.py, changing logging from error to warning reduces visibility of potential security issues, which could lead to missed critical information.

Vulnerability	Potential Incomplete CVSS Validation
Description	In dojo/models.py, the custom validator might introduce less strict validation rules compared to the original regex, potentially allowing invalid CVSS vectors.

Vulnerability	Potential Data Sanitization Issue
Description	In dojo/models.py, the new parse_cvss_data() function might have incomplete input validation, risking potential data manipulation.

Vulnerability	Potential Information Disclosure through CVSS Parsing
Description	In dojo/utils.py, the new CVSS parsing function could expose sensitive vulnerability scoring information.

Vulnerability	Removal of Tag Validation Logic
Description	In dojo/utils.py, elimination of previous input validation for tags could introduce sanitization risks and potential data integrity issues.

Vulnerability	Potential Logging of Sensitive Input
Description	In dojo/validators.py, error logging might expose sensitive vulnerability vector information.

Vulnerability	Broad Exception Handling
Description	In dojo/validators.py, the configurable exception class could allow manipulation of error handling, potentially bypassing security controls.

Vulnerability	Potential Lack of Input Validation for CVSSv3 Vectors
Description	In unittests/test_finding_model.py, saving vectors without validation could lead to data integrity issues and potential misrepresentation of vulnerability severity.

Vulnerability	Insufficient CVSS Vector Validation
Description	In unittests/test_rest_framework.py, previously allowed storing invalid CVSS vectors, potentially misrepresenting vulnerability severity and compromising risk assessment.

---

All finding details can be found in the DryRun Security Dashboard.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

Conflicts have been resolved. A maintainer will review the pull request shortly.

[Maffooch]

This is looking really good - only a couple questions/nits

[Maffooch]

I think we should remove invalid vectors, but only if the id is None before the finding is saved. That allows us to prevent the issue from spreading even further without implications for existing data

[valentijnscholten]

addressed this, but maybe we should just throw a ValidationError? That is what will happen in the future if we implement better validation support for Findings in general via clean.

[Maffooch]

actually, I think the validator will do this already, as its called before the save method is, right?