VULNERABILITY.SEVERITY should be updated in database

[rbt-mm]

Current Behavior

If the severity of a Vulnerability is NULL in the database, the API will calculate a severity before responding with a Vulnerability object.

dependency-track/src/main/java/org/dependencytrack/util/VulnerabilityUtil.java

Lines 75 to 95 in ce2267f

	public static Severity getSeverity(final Object severity, final BigDecimal cvssV2BaseScore, final BigDecimal cvssV3BaseScore, final BigDecimal owaspRRLikelihoodScore, final BigDecimal owaspRRTechnicalImpactScore, final BigDecimal owaspRRBusinessImpactScore) {
	if (severity instanceof String) {
	final String s = (String)severity;
	if (s.equalsIgnoreCase(Severity.CRITICAL.name())) {
	return Severity.CRITICAL;
	} else if (s.equalsIgnoreCase(Severity.HIGH.name())) {
	return Severity.HIGH;
	} else if (s.equalsIgnoreCase(Severity.MEDIUM.name())) {
	return Severity.MEDIUM;
	} else if (s.equalsIgnoreCase(Severity.LOW.name())) {
	return Severity.LOW;
	} else if (s.equalsIgnoreCase(Severity.INFO.name())) {
	return Severity.INFO;
	}
	} else if (severity instanceof Severity) {
	return (Severity)severity;
	} else {
	return getSeverity(cvssV2BaseScore, cvssV3BaseScore, owaspRRLikelihoodScore, owaspRRTechnicalImpactScore, owaspRRBusinessImpactScore);
	}
	return Severity.UNASSIGNED;
	}

This creates a mismatch between the database and the API responses, because the severity value will still remain NULL in the database, even though a severity for this vulnerability is now known after the calculation.

Example of mismatch between database and API/Frontend

Database entry:

API response:

Frontend:

This mismatch makes correctly filtering the vulnerabilities in the database by severity and responding via API with the filtered list impossible. The calculation cannot be performed by the database and therefore it will filter vulnerabilities that normally would contain the queried severity in the response, because those vulnerabilities do not have a severity in the database.

Proposed Behavior

The severity value of a Vulnerability shouldn't be NULL in the database.

It should either be updated after calculation to the calculated value or, probably even better, the severity should be calculated before creating a new vulnerability and be directly inserted with the vulnerability in the database to minimize the amount of mismatches and make correctly filtering by severity possible in the database.

Checklist

• I have read and understand the contributing guidelines
• I have checked the existing issues for whether this enhancement was already requested

[stevespringett]

The severity value of a Vulnerability shouldn't be NULL in the database.

This is how DT knowns if a vuln has an unassigned severity. We could always populate this field with 'Unassigned", so that its no longer null, but it would not change the behavior.

Users do not consume the database directly, only the API. If there are irregularities in the API, then thats something we should fix. But the database is not designed to be directly consumed. I don't want to add more logic to this or additional fields to indicate a null value as those would introduce many changes to logic across the backend and frontend that does not benefit the end user.

[rkg-mm]

@stevespringett We stumpled across this issue when implementing the API for
#2472

Any alternative ideas how to solve this?
Putting "unassigned" would not work, unless we include those in all querys, and do a manual filtering on these after loading them from DB, calculating all their "dynamic" values and removing the ones that should not be included. Possible but an ugly hack...

[nscuro]

I feel like the logic of calculating the severity could be shifted to the point in time when the vulnerability record is updated in the database. If we only calculate it at runtime, it's impossible to query by severity, resulting in very inefficient fetching behavior.

Whether or not a vuln has an unassigned severity is a check that can be done just as well at "persist-time".

We'd have to make sure though that existing data is not negatively affected by this change, i.e. we need either a migration that populates the severity field for all existing vulns, or Vulnerability#getSeverity can handle both new and old behavior.

[rkg-mm]

@nscuro is there a negative impact if the current field is just set with a calculated value? Like could the calculation change at some point? Would it be necessary to mark the value as calculated?

[nscuro]

@rkg-mm Like could the calculation change at some point?

It has not changed for >5 years, looking at the code. However, the calculation is priority-based, where the priorities are:

1. Predefined severity from DB
2. CVSSv3 (includes v3.1)
3. CVSSv2
4. OWASP RR
   • When both CVSSvX and OWASP RR are available, the highest severity of the two is picked

The score -> severity mappings are clearly defined for both CVSS and OWASP RR, so those won't change.

That being said, CVSSv4 is already available, and CycloneDX v1.5 supports it, too. Once DT supports it, the prioritization mentioned above may change such that CVSSv4 will be preferred over CVSSv3.x.

But, unless CVSSv4 support is added, we can't ingest it anyway. Which means we will not have to re-compute all the severities in the portfolio, it will still suffice to apply the new priority when a vulnerability record with CVSSv4 rating is updated.

Outside of that, if we change the computation, then yes, I guess we would have to recompute all values. But even that would be a one-time thing that we could do either through upgrades, or a task that corrects data in the background. After all, we're talking about a finite number of vulnerability records (~200-400k, depending how many sources are enabled). And again, I don't see this scenario happening really.

Would it be necessary to mark the value as calculated?

I don't currently see any benefit in that. FWIW, I see the following cases when a vulnerability is created / updated:

• No scoring is available, no severity specified: UNASSIGNED
• No scoring is available, severity explicitly specified: Use specified severity
• Scoring is available, no severity specified: Compute severity based on scores (see priorities above)
• Scoring is available, severity explicitly specified: Compute severity based on scores (see priorities above)
• No scoring is available, severity is explicitly UNASSIGNED: UNASSIGNED
• Scoring is available but invalid / unknown, no severity specified: UNASSIGNED
• Scoring is available but invalid / unknown, severity explicitly specified: Use specified severity

If, for some reason, we still can't store the calculated severity in the DB, it's still possible to perform the same computation in SQL. It's pretty much a giant CASE WHEN statement. For Hyades, we moved the metrics calculation to stored procedures, and had to do the same: https://github.com/DependencyTrack/hyades-apiserver/blob/8dc6d1e20039836d94e3e574982f4d817826181c/src/main/resources/storedprocs-postgres.sql#L1-L62

[rkg-mm]

Ok thank you, in this case I agree the calculation should be moved to the moment the vulnerability is either created or updated from any database, and the calculation stored in the existing field.

[rkg-mm]

My team is currently working on an implementation for this :)

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.