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 reveals multiple security vulnerabilities across different files, including potential denial of service risks from CVSS parsing, command injection in a unittest script, information disclosure through logging, and inconsistent vulnerability scoring that could misrepresent security severity.

⚠️ Potential Malicious Input Processing in dojo/tools/npm_audit_7_plus/parser.py

Vulnerability	Potential Malicious Input Processing
Description	Multiple parsers (Aqua, npm audit, JFrog Xray) introduce parsing of external, untrusted CVSS vector data using parse_cvss_data. While the function is likely designed to be robust, processing external input without comprehensive validation could lead to unexpected behavior or resource consumption issues if the input is maliciously crafted.

django-DefectDojo/dojo/tools/npm_audit_7_plus/parser.py

Lines 3 to 9 in 1a522af

	import logging
	from dojo.models import Finding
	from dojo.utils import parse_cvss_data
	logger = logging.getLogger(__name__)

⚠️ CVSS Score Inconsistency in dojo/tools/qualys/parser.py

Vulnerability	CVSS Score Inconsistency
Description	In the Qualys parser, the code unconditionally overwrites the CVSSv3 score derived from the vector with a potentially different CVSS_value. This could lead to incorrect severity representation if the scores from different sources (v2 vs v3) are mixed, potentially misrepresenting the true vulnerability severity.

django-DefectDojo/dojo/tools/qualys/parser.py

Lines 8 to 14 in 1a522af

	from dojo.models import Endpoint, Finding
	from dojo.tools.qualys import csv_parser
	from dojo.utils import parse_cvss_data
	logger = logging.getLogger(__name__)

⚠️ Potential Denial of Service via CVSS Parsing in dojo/utils.py

Vulnerability	Potential Denial of Service via CVSS Parsing
Description	The parse_cvss_data function in dojo/utils.py passes cvss_vector_string directly to cvss.parser.parse_cvss_from_text without input validation. A maliciously crafted CVSS vector string could potentially cause excessive resource consumption during parsing, leading to a denial of service condition.

django-DefectDojo/dojo/utils.py

Lines 15 to 32 in 1a522af

	import bleach
	import crum
	import cvss.parser
	import hyperlink
	import vobject
	from asteval import Interpreter
	from cryptography.hazmat.backends import default_backend
	from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes
	from cvss.cvss3 import CVSS3
	from dateutil.parser import parse
	from dateutil.relativedelta import MO, SU, relativedelta
	from django.conf import settings
	from django.contrib import messages
	from django.contrib.auth.signals import user_logged_in, user_logged_out, user_login_failed
	from django.core.paginator import Paginator
	from django.db.models import Case, Count, IntegerField, Q, Sum, Value, When
	from django.db.models.query import QuerySet

⚠️ Information Disclosure via Logging in dojo/validators.py

Vulnerability	Information Disclosure via Logging
Description	The tag_validator and cvss3_validator functions log user-controlled input directly, which could expose sensitive details or aid attacker reconnaissance. Specifically, logger.error("cvss3_validator called with value: %s", value) and the error message construction in tag_validator embed unvalidated user input in log messages and error responses.

django-DefectDojo/dojo/validators.py

Lines 1 to 53 in 1a522af

	import logging
	import re
	from collections.abc import Callable
	import cvss.parser
	from cvss import CVSS2, CVSS3, CVSS4
	from django.core.exceptions import ValidationError
	logger = logging.getLogger(__name__)
	def tag_validator(value: str | list[str], exception_class: Callable = ValidationError) -> None:
	TAG_PATTERN = re.compile(r'[ ,\'"]')
	error_messages = []
	if isinstance(value, list):
	error_messages.extend(f"Invalid tag: '{tag}'. Tags should not contain spaces, commas, or quotes." for tag in value if TAG_PATTERN.search(tag))
	elif isinstance(value, str):
	if TAG_PATTERN.search(value):
	error_messages.append(f"Invalid tag: '{value}'. Tags should not contain spaces, commas, or quotes.")
	else:
	error_messages.append(f"Value must be a string or list of strings: {value} - {type(value)}.")
	if error_messages:
	logger.debug(f"Tag validation failed: {error_messages}")
	raise exception_class(error_messages)
	def cvss3_validator(value: str | list[str], exception_class: Callable = ValidationError) -> None:
	logger.error("cvss3_validator called with value: %s", value)
	cvss_vectors = cvss.parser.parse_cvss_from_text(value)
	if len(cvss_vectors) > 0:
	vector_obj = cvss_vectors[0]
	if isinstance(vector_obj, CVSS3):
	# all is good
	return
	if isinstance(vector_obj, CVSS4):
	# CVSS4 is not supported yet by the parse_cvss_from_text function, but let's prepare for it anyway: https://github.com/RedHatProductSecurity/cvss/issues/53
	msg = "Unsupported CVSS(4) version detected."
	raise exception_class(msg)
	if isinstance(vector_obj, CVSS2):
	msg = "Unsupported CVSS(2) version detected."
	raise exception_class(msg)
	msg = "Unsupported CVSS version detected."
	raise exception_class(msg)
	# Explicitly raise an error if no CVSS vectors are found,
	# to avoid 'NoneType' errors during severity processing later.
	msg = "No valid CVSS vectors found by cvss.parse_cvss_from_text()"
	raise exception_class(msg)

⚠️ Command Injection Risk in run-unittest.sh

Vulnerability

Command Injection Risk

Description

The run-unittest.sh script executes a shell command using the user-controlled $TEST_CASE variable without proper sanitization. An attacker could manipulate the TEST_CASE input to inject arbitrary shell commands, potentially leading to remote code execution. The command docker compose exec uwsgi bash -c "python manage.py test $TEST_CASE -v2 --keepdb" directly interpolates user input into a shell command without escaping.

django-DefectDojo/run-unittest.sh

Lines 51 to 57 in 1a522af

	fi
	echo "Running docker compose unit tests with test case $TEST_CASE ..."
	# Compose V2 integrates compose functions into the Docker platform, continuing to support
	# most of the previous docker-compose features and flags. You can run Compose V2 by
	# replacing the hyphen (-) with a space, using docker compose, instead of docker-compose.
	docker compose exec uwsgi bash -c "python manage.py test $TEST_CASE -v2 --keepdb"

---

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?

[valentijnscholten]

I believe only when setting a CVSS3 vector through the UI or API, but not when a (re)import is done.

[Maffooch]

Let's get this in dev for a bit and see where we land