From 47105cdb426f8ea204b96798758bf6eca5d8a86b Mon Sep 17 00:00:00 2001 From: Niklas Date: Thu, 13 Jun 2024 17:11:10 +0200 Subject: [PATCH] Store computed severities in the database (#706) --- .../VulnerabilityScanResultProcessor.java | 3 - .../dependencytrack/model/Vulnerability.java | 28 +--- .../ModelConverterCdxToVuln.java | 9 ++ .../jdbi/NotificationSubjectDao.java | 23 +--- .../change/v550/ComputeSeveritiesChange.java | 125 ++++++++++++++++++ .../resources/v1/VulnerabilityResource.java | 15 ++- .../migration/changelog-procedures.xml | 9 -- .../resources/migration/changelog-v5.5.0.xml | 9 ++ .../procedures/function_calc-severity.sql | 27 ---- .../function_cvssv2-to-severity.sql | 17 --- .../function_cvssv3-to-severity.sql | 18 --- .../procedure_update-component-metrics.sql | 16 +-- .../model/VulnerabilityTest.java | 23 ++-- .../model/mapping/PolicyProtoMapperTest.java | 8 +- .../NotificationModelConverterTest.java | 2 + .../jdbi/NotificationSubjectDaoTest.java | 6 +- .../v550/ComputeSeveritiesChangeTest.java | 112 ++++++++++++++++ .../ComputeSeveritiesChangeTest-changelog.xml | 14 ++ .../ComputeSeveritiesChangeTest-schema.sql | 17 +++ 19 files changed, 327 insertions(+), 154 deletions(-) create mode 100644 src/main/java/org/dependencytrack/persistence/migration/change/v550/ComputeSeveritiesChange.java delete mode 100644 src/main/resources/migration/procedures/function_calc-severity.sql delete mode 100644 src/main/resources/migration/procedures/function_cvssv2-to-severity.sql delete mode 100644 src/main/resources/migration/procedures/function_cvssv3-to-severity.sql create mode 100644 src/test/java/org/dependencytrack/persistence/migration/change/v550/ComputeSeveritiesChangeTest.java create mode 100644 src/test/resources/migration/custom/ComputeSeveritiesChangeTest-changelog.xml create mode 100644 src/test/resources/migration/custom/ComputeSeveritiesChangeTest-schema.sql diff --git a/src/main/java/org/dependencytrack/event/kafka/processor/VulnerabilityScanResultProcessor.java b/src/main/java/org/dependencytrack/event/kafka/processor/VulnerabilityScanResultProcessor.java index 51752c586..49cc4ef7c 100644 --- a/src/main/java/org/dependencytrack/event/kafka/processor/VulnerabilityScanResultProcessor.java +++ b/src/main/java/org/dependencytrack/event/kafka/processor/VulnerabilityScanResultProcessor.java @@ -334,9 +334,6 @@ private Vulnerability syncVulnerability(final QueryManager qm, final Vulnerabili differ.applyIfChanged("published", Vulnerability::getPublished, existingVuln::setPublished); differ.applyIfChanged("updated", Vulnerability::getUpdated, existingVuln::setUpdated); differ.applyIfChanged("cwes", Vulnerability::getCwes, existingVuln::setCwes); - // Calling setSeverity nulls all CVSS and OWASP RR fields. getSeverity calculates the severity on-the-fly, - // and will return UNASSIGNED even when no severity is set explicitly. Thus, calling setSeverity - // must happen before CVSS and OWASP RR fields are set, to avoid null-ing them again. differ.applyIfChanged("severity", Vulnerability::getSeverity, existingVuln::setSeverity); differ.applyIfChanged("cvssV2BaseScore", Vulnerability::getCvssV2BaseScore, existingVuln::setCvssV2BaseScore); differ.applyIfChanged("cvssV2ImpactSubScore", Vulnerability::getCvssV2ImpactSubScore, existingVuln::setCvssV2ImpactSubScore); diff --git a/src/main/java/org/dependencytrack/model/Vulnerability.java b/src/main/java/org/dependencytrack/model/Vulnerability.java index a284dd167..2c3a8cd84 100644 --- a/src/main/java/org/dependencytrack/model/Vulnerability.java +++ b/src/main/java/org/dependencytrack/model/Vulnerability.java @@ -32,7 +32,6 @@ import org.dependencytrack.resources.v1.serializers.CweSerializer; import org.dependencytrack.resources.v1.serializers.Iso8601DateSerializer; import org.dependencytrack.resources.v1.vo.AffectedComponent; -import org.dependencytrack.util.VulnerabilityUtil; import javax.jdo.annotations.Column; import javax.jdo.annotations.Convert; @@ -349,36 +348,11 @@ public void setId(long id) { this.id = id; } - /** - * Returns the value of the severity field (if specified), otherwise, will - * return the severity based on the numerical CVSS or OWASP score. - * - * This method properly accounts for vulnerabilities that may have a subset or all of (CVSSv2, CVSSv3, OWASP RR) - * score. The highest severity is returned. - * @return the severity of the vulnerability - * @see VulnerabilityUtil#getSeverity(BigDecimal, BigDecimal, BigDecimal, BigDecimal, BigDecimal) - */ public Severity getSeverity() { - return (this.severity != null) ? severity : VulnerabilityUtil.getSeverity(cvssV2BaseScore, cvssV3BaseScore, owaspRRLikelihoodScore, owaspRRTechnicalImpactScore, owaspRRBusinessImpactScore); + return severity; } - /** - * Sets the severity. This should only be set if CVSSv2, CVSSv3 or OWASP RR scores - * are not used. - * @param severity the severity of the vulnerability - */ public void setSeverity(Severity severity) { - cvssV2BaseScore = null; - cvssV2ImpactSubScore = null; - cvssV2ExploitabilitySubScore = null; - cvssV2Vector = null; - cvssV3BaseScore = null; - cvssV3ImpactSubScore = null; - cvssV3ExploitabilitySubScore = null; - cvssV3Vector = null; - owaspRRLikelihoodScore = null; - owaspRRBusinessImpactScore = null; - owaspRRTechnicalImpactScore = null; this.severity = severity; } diff --git a/src/main/java/org/dependencytrack/parser/dependencytrack/ModelConverterCdxToVuln.java b/src/main/java/org/dependencytrack/parser/dependencytrack/ModelConverterCdxToVuln.java index 5676a28cc..b18e34cd7 100644 --- a/src/main/java/org/dependencytrack/parser/dependencytrack/ModelConverterCdxToVuln.java +++ b/src/main/java/org/dependencytrack/parser/dependencytrack/ModelConverterCdxToVuln.java @@ -31,6 +31,7 @@ import org.dependencytrack.parser.common.resolver.CweResolver; import org.dependencytrack.persistence.QueryManager; import org.dependencytrack.proto.vulnanalysis.v1.Scanner; +import org.dependencytrack.util.VulnerabilityUtil; import us.springett.cvss.Cvss; import us.springett.cvss.Score; import us.springett.owasp.riskrating.MissingFactorException; @@ -170,6 +171,14 @@ public static Vulnerability convert(final QueryManager qm, final Bom bom, } } } + vuln.setSeverity(VulnerabilityUtil.getSeverity( + vuln.getSeverity(), + vuln.getCvssV2BaseScore(), + vuln.getCvssV3BaseScore(), + vuln.getOwaspRRLikelihoodScore(), + vuln.getOwaspRRTechnicalImpactScore(), + vuln.getOwaspRRBusinessImpactScore() + )); // There can be cases where ratings do not have a known method, and the source only assigned // a severity. Such ratings are inferior to those with proper method and vector, but we'll use diff --git a/src/main/java/org/dependencytrack/persistence/jdbi/NotificationSubjectDao.java b/src/main/java/org/dependencytrack/persistence/jdbi/NotificationSubjectDao.java index 56f08d9a8..47a2a92dc 100644 --- a/src/main/java/org/dependencytrack/persistence/jdbi/NotificationSubjectDao.java +++ b/src/main/java/org/dependencytrack/persistence/jdbi/NotificationSubjectDao.java @@ -130,12 +130,7 @@ public interface NotificationSubjectDao extends SqlObject { WHEN "A"."SEVERITY" IS NOT NULL THEN "A"."OWASPVECTOR" ELSE "V"."OWASPRRVECTOR" END AS "vulnOwaspRrVector", - "CALC_SEVERITY"( - "V"."SEVERITY", - "A"."SEVERITY", - "V"."CVSSV3BASESCORE", - "V"."CVSSV2BASESCORE" - ) AS "vulnSeverity", + COALESCE("A"."SEVERITY", "V"."SEVERITY") AS "vulnSeverity", STRING_TO_ARRAY("V"."CWES", ',') AS "vulnCwes", "vulnAliasesJson", :vulnAnalysisLevel AS "vulnAnalysisLevel", @@ -249,12 +244,7 @@ List getForNewVulnerabilities(final UUID componentUuid, WHEN "A"."SEVERITY" IS NOT NULL THEN "A"."OWASPVECTOR" ELSE "V"."OWASPRRVECTOR" END AS "vulnOwaspRrVector", - "CALC_SEVERITY"( - "V"."SEVERITY", - "A"."SEVERITY", - "V"."CVSSV3BASESCORE", - "V"."CVSSV2BASESCORE" - ) AS "vulnSeverity", + COALESCE("A"."SEVERITY", "V"."SEVERITY") AS "vulnSeverity", STRING_TO_ARRAY("V"."CWES", ',') AS "vulnCwes", "vulnAliasesJson" FROM @@ -364,12 +354,7 @@ LEFT JOIN LATERAL ( WHEN "A"."SEVERITY" IS NOT NULL THEN "A"."OWASPVECTOR" ELSE "V"."OWASPRRVECTOR" END AS "vulnOwaspRrVector", - "CALC_SEVERITY"( - "V"."SEVERITY", - "A"."SEVERITY", - "V"."CVSSV3BASESCORE", - "V"."CVSSV2BASESCORE" - ) AS "vulnSeverity", + COALESCE("A"."SEVERITY", "V"."SEVERITY") AS "vulnSeverity", STRING_TO_ARRAY("V"."CWES", ',') AS "vulnCwes", "vulnAliasesJson", :isSuppressed AS "isVulnAnalysisSuppressed", @@ -519,7 +504,7 @@ default Optional getForProjectVulnAnalysisCo THEN "A"."OWASPVECTOR" ELSE "V"."OWASPRRVECTOR" END AS "vulnOwaspRrVector" - , "CALC_SEVERITY"("V"."SEVERITY", "A"."SEVERITY", "V"."CVSSV3BASESCORE", "V"."CVSSV2BASESCORE") AS "vulnSeverity" + , COALESCE("A"."SEVERITY", "V"."SEVERITY") AS "vulnSeverity" , STRING_TO_ARRAY("V"."CWES", ',') AS "vulnCwes" , "vulnAliasesJson" FROM "COMPONENT" AS "C" diff --git a/src/main/java/org/dependencytrack/persistence/migration/change/v550/ComputeSeveritiesChange.java b/src/main/java/org/dependencytrack/persistence/migration/change/v550/ComputeSeveritiesChange.java new file mode 100644 index 000000000..8f3bd9f93 --- /dev/null +++ b/src/main/java/org/dependencytrack/persistence/migration/change/v550/ComputeSeveritiesChange.java @@ -0,0 +1,125 @@ +/* + * This file is part of Dependency-Track. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + * Copyright (c) OWASP Foundation. All Rights Reserved. + */ +package org.dependencytrack.persistence.migration.change.v550; + +import liquibase.change.custom.CustomTaskChange; +import liquibase.database.Database; +import liquibase.database.jvm.JdbcConnection; +import liquibase.exception.CustomChangeException; +import liquibase.exception.DatabaseException; +import liquibase.exception.SetupException; +import liquibase.exception.ValidationErrors; +import liquibase.resource.ResourceAccessor; +import org.dependencytrack.model.Severity; +import org.dependencytrack.util.VulnerabilityUtil; + +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; + +public class ComputeSeveritiesChange implements CustomTaskChange { + + private int batchSize; + private int numBatches; + private int numUpdates; + + @Override + public void setUp() throws SetupException { + } + + @Override + public void execute(final Database database) throws CustomChangeException { + final var connection = (JdbcConnection) database.getConnection(); + + // NB: When generating the schema via `mvn liquibase:updateSQL`, none of the changesets + // is actually applied. If we don't perform a preliminary check here, schema generation fails. + try (final PreparedStatement ps = connection.prepareStatement(""" + SELECT 1 + FROM information_schema.tables + WHERE table_schema = current_schema() + AND table_name = 'VULNERABILITY' + """)) { + if (!ps.executeQuery().next()) { + // Probably running within `mvn liquibase:updateSQL`. + return; + } + } catch (DatabaseException | SQLException e) { + throw new CustomChangeException("Failed to check for databasechangelog table", e); + } + + try (final PreparedStatement selectStatement = connection.prepareStatement(""" + SELECT "CVSSV2BASESCORE" + , "CVSSV3BASESCORE" + , "OWASPRRLIKELIHOODSCORE" + , "OWASPRRTECHNICALIMPACTSCORE" + , "OWASPRRBUSINESSIMPACTSCORE" + , "VULNID" + FROM "VULNERABILITY" + WHERE "SEVERITY" IS NULL + """); + final PreparedStatement updateStatement = connection.prepareStatement(""" + UPDATE "VULNERABILITY" SET "SEVERITY" = ? WHERE "VULNID" = ? + """)) { + final ResultSet rs = selectStatement.executeQuery(); + while (rs.next()) { + final String vulnId = rs.getString(6); + final Severity severity = VulnerabilityUtil.getSeverity( + rs.getBigDecimal(1), + rs.getBigDecimal(2), + rs.getBigDecimal(3), + rs.getBigDecimal(4), + rs.getBigDecimal(5) + ); + + updateStatement.setString(1, severity.name()); + updateStatement.setString(2, vulnId); + updateStatement.addBatch(); + if (++batchSize == 500) { + updateStatement.executeBatch(); + numUpdates += batchSize; + numBatches++; + batchSize = 0; + } + } + + if (batchSize > 0) { + updateStatement.executeBatch(); + numUpdates += batchSize; + numBatches++; + } + } catch (DatabaseException | SQLException e) { + throw new CustomChangeException("Failed to update severities", e); + } + } + + @Override + public String getConfirmationMessage() { + return "Updated %d vulnerabilities in %d batches".formatted(numUpdates, numBatches); + } + + @Override + public void setFileOpener(final ResourceAccessor resourceAccessor) { + } + + @Override + public ValidationErrors validate(final Database database) { + return null; + } + +} diff --git a/src/main/java/org/dependencytrack/resources/v1/VulnerabilityResource.java b/src/main/java/org/dependencytrack/resources/v1/VulnerabilityResource.java index 455c1932a..16e499774 100644 --- a/src/main/java/org/dependencytrack/resources/v1/VulnerabilityResource.java +++ b/src/main/java/org/dependencytrack/resources/v1/VulnerabilityResource.java @@ -312,7 +312,7 @@ public Response createVulnerability(Vulnerability jsonVulnerability) { } } } - recalculateScoresFromVector(jsonVulnerability); + recalculateScoresAndSeverityFromVectors(jsonVulnerability); jsonVulnerability.setSource(Vulnerability.Source.INTERNAL); vulnerability = qm.createVulnerability(jsonVulnerability, true); qm.persist(vsList); @@ -391,7 +391,7 @@ public Response updateVulnerability(Vulnerability jsonVuln) { final List resolvedTags = qm.resolveTags(jsonVuln.getTags()); qm.bind(vulnerability, resolvedTags); - recalculateScoresFromVector(jsonVuln); + recalculateScoresAndSeverityFromVectors(jsonVuln); vulnerability = qm.updateVulnerability(jsonVuln, true); qm.persist(vsList); vsList = qm.reconcileVulnerableSoftware(vulnerability, vsListOld, vsList, Vulnerability.Source.INTERNAL); @@ -460,7 +460,7 @@ public Response generateInternalVulnerabilityIdentifier() { return Response.ok(vulnId).build(); } - public void recalculateScoresFromVector(Vulnerability vuln) throws MissingFactorException { + public void recalculateScoresAndSeverityFromVectors(Vulnerability vuln) throws MissingFactorException { // Recalculate V2 score based on vector passed to resource and normalize vector final Cvss v2 = Cvss.fromVector(vuln.getCvssV2Vector()); if (v2 != null) { @@ -489,6 +489,15 @@ public void recalculateScoresFromVector(Vulnerability vuln) throws MissingFactor vuln.setOwaspRRTechnicalImpactScore(BigDecimal.valueOf(score.getTechnicalImpactScore())); vuln.setOwaspRRBusinessImpactScore(BigDecimal.valueOf(score.getBusinessImpactScore())); } + + vuln.setSeverity(VulnerabilityUtil.getSeverity( + vuln.getSeverity(), + vuln.getCvssV2BaseScore(), + vuln.getCvssV3BaseScore(), + vuln.getOwaspRRLikelihoodScore(), + vuln.getOwaspRRTechnicalImpactScore(), + vuln.getOwaspRRBusinessImpactScore() + )); } @POST diff --git a/src/main/resources/migration/changelog-procedures.xml b/src/main/resources/migration/changelog-procedures.xml index 1f1e1bc2f..9eec6ff19 100644 --- a/src/main/resources/migration/changelog-procedures.xml +++ b/src/main/resources/migration/changelog-procedures.xml @@ -8,15 +8,6 @@ http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-ext.xsd http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-latest.xsd"> - - - - - - - - - diff --git a/src/main/resources/migration/changelog-v5.5.0.xml b/src/main/resources/migration/changelog-v5.5.0.xml index 70b215c82..395fb7d61 100644 --- a/src/main/resources/migration/changelog-v5.5.0.xml +++ b/src/main/resources/migration/changelog-v5.5.0.xml @@ -141,4 +141,13 @@ CHECK ("PROPERTYTYPE" IS NULL OR "PROPERTYTYPE"::TEXT = ANY(ARRAY['BOOLEAN', 'INTEGER', 'NUMBER', 'STRING', 'ENCRYPTEDSTRING', 'TIMESTAMP', 'URL', 'UUID'])); + + + + + DROP FUNCTION IF EXISTS "CVSSV2_TO_SEVERITY"; + DROP FUNCTION IF EXISTS "CVSSV3_TO_SEVERITY"; + DROP FUNCTION IF EXISTS "CALC_SEVERITY"; + + \ No newline at end of file diff --git a/src/main/resources/migration/procedures/function_calc-severity.sql b/src/main/resources/migration/procedures/function_calc-severity.sql deleted file mode 100644 index 826a961d8..000000000 --- a/src/main/resources/migration/procedures/function_calc-severity.sql +++ /dev/null @@ -1,27 +0,0 @@ --- Calculate the severity of a vulnerability based on: --- * a pre-set severity --- * a CVSSv3 base score --- * a CVSSv2 base score --- The behavior of this function is identical to Vulnerability#getSeverity --- in the API server Java code base. --- https://github.com/DependencyTrack/dependency-track/blob/1976be1f5cc9d027900f09aed9d1539595aeda3a/src/main/java/org/dependencytrack/model/Vulnerability.java#L338-L340 -CREATE OR REPLACE FUNCTION "CALC_SEVERITY"( - "severity" VARCHAR, - "severity_override" VARCHAR, - "cvssv3_base_score" NUMERIC, - "cvssv2_base_score" NUMERIC -) RETURNS VARCHAR - LANGUAGE "sql" - PARALLEL SAFE - IMMUTABLE -AS -$$ -SELECT - CASE - WHEN "severity_override" IS NOT NULL THEN "severity_override" - WHEN "cvssv3_base_score" IS NOT NULL THEN "CVSSV3_TO_SEVERITY"("cvssv3_base_score") - WHEN "cvssv2_base_score" IS NOT NULL THEN "CVSSV2_TO_SEVERITY"("cvssv2_base_score") - WHEN "severity" IS NOT NULL THEN "severity" - ELSE 'UNASSIGNED' - END; -$$; \ No newline at end of file diff --git a/src/main/resources/migration/procedures/function_cvssv2-to-severity.sql b/src/main/resources/migration/procedures/function_cvssv2-to-severity.sql deleted file mode 100644 index ca982c6b1..000000000 --- a/src/main/resources/migration/procedures/function_cvssv2-to-severity.sql +++ /dev/null @@ -1,17 +0,0 @@ --- Calculate the severity of a vulnerability based on its CVSSv2 base score. -CREATE OR REPLACE FUNCTION "CVSSV2_TO_SEVERITY"( - "base_score" NUMERIC -) RETURNS VARCHAR - LANGUAGE "sql" - PARALLEL SAFE - IMMUTABLE -AS -$$ -SELECT - CASE - WHEN "base_score" >= 7 THEN 'HIGH' - WHEN "base_score" >= 4 THEN 'MEDIUM' - WHEN "base_score" > 0 THEN 'LOW' - ELSE 'UNASSIGNED' - END; -$$ \ No newline at end of file diff --git a/src/main/resources/migration/procedures/function_cvssv3-to-severity.sql b/src/main/resources/migration/procedures/function_cvssv3-to-severity.sql deleted file mode 100644 index 4bf47cd13..000000000 --- a/src/main/resources/migration/procedures/function_cvssv3-to-severity.sql +++ /dev/null @@ -1,18 +0,0 @@ --- Calculate the severity of a vulnerability based on its CVSSv3 base score. -CREATE OR REPLACE FUNCTION "CVSSV3_TO_SEVERITY"( - "base_score" NUMERIC -) RETURNS VARCHAR - LANGUAGE "sql" - PARALLEL SAFE - IMMUTABLE -AS -$$ -SELECT - CASE - WHEN "base_score" >= 9 THEN 'CRITICAL' - WHEN "base_score" >= 7 THEN 'HIGH' - WHEN "base_score" >= 4 THEN 'MEDIUM' - WHEN "base_score" > 0 THEN 'LOW' - ELSE 'UNASSIGNED' - END; -$$; \ No newline at end of file diff --git a/src/main/resources/migration/procedures/procedure_update-component-metrics.sql b/src/main/resources/migration/procedures/procedure_update-component-metrics.sql index 8ca0acbeb..bbf3847fc 100644 --- a/src/main/resources/migration/procedures/procedure_update-component-metrics.sql +++ b/src/main/resources/migration/procedures/procedure_update-component-metrics.sql @@ -9,7 +9,6 @@ DECLARE "v_vulnerability" RECORD; -- Loop variable for iterating over vulnerabilities the component is affected by "v_alias" RECORD; -- Loop variable for iterating over aliases of a vulnerability "v_aliases_seen" TEXT[]; -- Array of aliases encountered while iterating over vulnerabilities - "v_severity" VARCHAR; -- Loop variable for the current vulnerability's severity "v_policy_violation" RECORD; -- Loop variable for iterating over policy violations assigned to the component "v_vulnerabilities" INT := 0; -- Total number of vulnerabilities "v_critical" INT := 0; -- Number of vulnerabilities with critical severity @@ -99,20 +98,13 @@ BEGIN "v_vulnerabilities" := "v_vulnerabilities" + 1; - SELECT "CALC_SEVERITY"( - "v_vulnerability"."SEVERITY", - "v_vulnerability"."SEVERITY_OVERRIDE", - "v_vulnerability"."CVSSV3BASESCORE", - "v_vulnerability"."CVSSV2BASESCORE") - INTO "v_severity"; - - IF "v_severity" = 'CRITICAL' THEN + IF COALESCE("v_vulnerability"."SEVERITY_OVERRIDE", "v_vulnerability"."SEVERITY") = 'CRITICAL' THEN "v_critical" := "v_critical" + 1; - ELSEIF "v_severity" = 'HIGH' THEN + ELSEIF COALESCE("v_vulnerability"."SEVERITY_OVERRIDE", "v_vulnerability"."SEVERITY") = 'HIGH' THEN "v_high" := "v_high" + 1; - ELSEIF "v_severity" = 'MEDIUM' THEN + ELSEIF COALESCE("v_vulnerability"."SEVERITY_OVERRIDE", "v_vulnerability"."SEVERITY") = 'MEDIUM' THEN "v_medium" := "v_medium" + 1; - ELSEIF "v_severity" = 'LOW' THEN + ELSEIF COALESCE("v_vulnerability"."SEVERITY_OVERRIDE", "v_vulnerability"."SEVERITY") = 'LOW' THEN "v_low" := "v_low" + 1; ELSE "v_unassigned" := "v_unassigned" + 1; diff --git a/src/test/java/org/dependencytrack/model/VulnerabilityTest.java b/src/test/java/org/dependencytrack/model/VulnerabilityTest.java index 1d11d4342..8a32733c7 100644 --- a/src/test/java/org/dependencytrack/model/VulnerabilityTest.java +++ b/src/test/java/org/dependencytrack/model/VulnerabilityTest.java @@ -38,6 +38,8 @@ public void testId() { @Test public void testSeverity() { + // NB: Before https://github.com/DependencyTrack/dependency-track/issues/2474, + // severity was computed in the getter based on the provided scores. Vulnerability vuln = new Vulnerability(); vuln.setCvssV2Vector("Vector"); vuln.setCvssV2BaseScore(new BigDecimal(5.0)); @@ -47,16 +49,19 @@ public void testSeverity() { vuln.setCvssV3BaseScore(new BigDecimal(6.0)); vuln.setCvssV3ImpactSubScore(new BigDecimal(6.1)); vuln.setCvssV3ExploitabilitySubScore(new BigDecimal(6.2)); - Assert.assertEquals(Severity.MEDIUM, vuln.getSeverity()); + Assert.assertNull(vuln.getSeverity()); + + // NB: Before https://github.com/DependencyTrack/dependency-track/issues/2474, + // calling setSeverity cleared all score and vector fields. vuln.setSeverity(Severity.HIGH); - Assert.assertNull(vuln.getCvssV2Vector()); - Assert.assertNull(vuln.getCvssV2BaseScore()); - Assert.assertNull(vuln.getCvssV2ImpactSubScore()); - Assert.assertNull(vuln.getCvssV2ExploitabilitySubScore()); - Assert.assertNull(vuln.getCvssV3Vector()); - Assert.assertNull(vuln.getCvssV3BaseScore()); - Assert.assertNull(vuln.getCvssV3ImpactSubScore()); - Assert.assertNull(vuln.getCvssV3ExploitabilitySubScore()); + Assert.assertNotNull(vuln.getCvssV2Vector()); + Assert.assertNotNull(vuln.getCvssV2BaseScore()); + Assert.assertNotNull(vuln.getCvssV2ImpactSubScore()); + Assert.assertNotNull(vuln.getCvssV2ExploitabilitySubScore()); + Assert.assertNotNull(vuln.getCvssV3Vector()); + Assert.assertNotNull(vuln.getCvssV3BaseScore()); + Assert.assertNotNull(vuln.getCvssV3ImpactSubScore()); + Assert.assertNotNull(vuln.getCvssV3ExploitabilitySubScore()); Assert.assertEquals(Severity.HIGH, vuln.getSeverity()); } diff --git a/src/test/java/org/dependencytrack/model/mapping/PolicyProtoMapperTest.java b/src/test/java/org/dependencytrack/model/mapping/PolicyProtoMapperTest.java index 1753f0508..77c455779 100644 --- a/src/test/java/org/dependencytrack/model/mapping/PolicyProtoMapperTest.java +++ b/src/test/java/org/dependencytrack/model/mapping/PolicyProtoMapperTest.java @@ -124,13 +124,7 @@ public void testMapVulnerabilityToProto() throws Exception { @Test public void testMapVulnerabilityWithNoFieldsSet() throws Exception { - assertThatJson(JsonFormat.printer().print(PolicyProtoMapper.mapToProto(new Vulnerability()))) - // Oddity of the Vulnerability behavior: getSeverity never returns null, but UNASSIGNED instead. - .isEqualTo(""" - { - "severity": "UNASSIGNED" - } - """); + assertThatJson(JsonFormat.printer().print(PolicyProtoMapper.mapToProto(new Vulnerability()))).isEqualTo("{}"); } @Test diff --git a/src/test/java/org/dependencytrack/parser/dependencytrack/NotificationModelConverterTest.java b/src/test/java/org/dependencytrack/parser/dependencytrack/NotificationModelConverterTest.java index dc561a0a1..af85b39da 100644 --- a/src/test/java/org/dependencytrack/parser/dependencytrack/NotificationModelConverterTest.java +++ b/src/test/java/org/dependencytrack/parser/dependencytrack/NotificationModelConverterTest.java @@ -23,6 +23,7 @@ import org.dependencytrack.model.Analysis; import org.dependencytrack.model.AnalysisState; import org.dependencytrack.model.Bom; +import org.dependencytrack.model.Severity; import org.dependencytrack.model.Tag; import org.dependencytrack.model.Vex; import org.dependencytrack.model.ViolationAnalysis; @@ -602,6 +603,7 @@ private org.dependencytrack.model.Vulnerability createVulnerability() { vuln.setOwaspRRLikelihoodScore(BigDecimal.valueOf(1.1)); vuln.setOwaspRRTechnicalImpactScore(BigDecimal.valueOf(2.2)); vuln.setOwaspRRBusinessImpactScore(BigDecimal.valueOf(3.3)); + vuln.setSeverity(Severity.MEDIUM); vuln.setCwes(List.of(666, 777)); return vuln; } diff --git a/src/test/java/org/dependencytrack/persistence/jdbi/NotificationSubjectDaoTest.java b/src/test/java/org/dependencytrack/persistence/jdbi/NotificationSubjectDaoTest.java index f0678f0ea..95f96121e 100644 --- a/src/test/java/org/dependencytrack/persistence/jdbi/NotificationSubjectDaoTest.java +++ b/src/test/java/org/dependencytrack/persistence/jdbi/NotificationSubjectDaoTest.java @@ -76,7 +76,7 @@ public void testGetForNewVulnerabilities() { vulnA.setSubTitle("vulnASubTitle"); vulnA.setDescription("vulnADescription"); vulnA.setRecommendation("vulnARecommendation"); - vulnA.setSeverity(Severity.MEDIUM); + vulnA.setSeverity(Severity.LOW); vulnA.setCvssV2BaseScore(BigDecimal.valueOf(1.1)); vulnA.setCvssV3BaseScore(BigDecimal.valueOf(2.2)); vulnA.setCvssV2Vector("(AV:N/AC:M/Au:S/C:P/I:P/A:P)"); @@ -304,7 +304,7 @@ public void testGetForNewVulnerableDependency() throws Exception { vulnA.setSubTitle("vulnASubTitle"); vulnA.setDescription("vulnADescription"); vulnA.setRecommendation("vulnARecommendation"); - vulnA.setSeverity(Severity.MEDIUM); + vulnA.setSeverity(Severity.LOW); vulnA.setCvssV2BaseScore(BigDecimal.valueOf(1.1)); vulnA.setCvssV3BaseScore(BigDecimal.valueOf(2.2)); vulnA.setCvssV2Vector("(AV:N/AC:M/Au:S/C:P/I:P/A:P)"); @@ -505,7 +505,7 @@ public void testGetForProjectAuditChange() { vulnA.setSubTitle("vulnASubTitle"); vulnA.setDescription("vulnADescription"); vulnA.setRecommendation("vulnARecommendation"); - vulnA.setSeverity(Severity.MEDIUM); + vulnA.setSeverity(Severity.LOW); vulnA.setCvssV2BaseScore(BigDecimal.valueOf(1.1)); vulnA.setCvssV3BaseScore(BigDecimal.valueOf(2.2)); vulnA.setCvssV2Vector("(AV:N/AC:M/Au:S/C:P/I:P/A:P)"); diff --git a/src/test/java/org/dependencytrack/persistence/migration/change/v550/ComputeSeveritiesChangeTest.java b/src/test/java/org/dependencytrack/persistence/migration/change/v550/ComputeSeveritiesChangeTest.java new file mode 100644 index 000000000..6b1af57ce --- /dev/null +++ b/src/test/java/org/dependencytrack/persistence/migration/change/v550/ComputeSeveritiesChangeTest.java @@ -0,0 +1,112 @@ +/* + * This file is part of Dependency-Track. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + * Copyright (c) OWASP Foundation. All Rights Reserved. + */ +package org.dependencytrack.persistence.migration.change.v550; + +import liquibase.Liquibase; +import liquibase.Scope; +import liquibase.command.CommandScope; +import liquibase.command.core.UpdateCommandStep; +import liquibase.command.core.helpers.DbUrlConnectionCommandStep; +import liquibase.database.Database; +import liquibase.database.DatabaseFactory; +import liquibase.database.jvm.JdbcConnection; +import liquibase.resource.ClassLoaderResourceAccessor; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.postgresql.ds.PGSimpleDataSource; +import org.testcontainers.containers.PostgreSQLContainer; +import org.testcontainers.utility.DockerImageName; + +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.util.Collections; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; + +public class ComputeSeveritiesChangeTest { + + private PostgreSQLContainer postgresContainer; + + @Before + @SuppressWarnings("resource") + public void setUp() { + postgresContainer = new PostgreSQLContainer<>(DockerImageName.parse("postgres:16-alpine")) + .withInitScript("migration/custom/ComputeSeveritiesChangeTest-schema.sql"); + postgresContainer.start(); + } + + @After + public void tearDown() { + if (postgresContainer != null) { + postgresContainer.stop(); + } + } + + @Test + public void test() throws Exception { + final var dataSource = new PGSimpleDataSource(); + dataSource.setUrl(postgresContainer.getJdbcUrl()); + dataSource.setUser(postgresContainer.getUsername()); + dataSource.setPassword(postgresContainer.getPassword()); + + try (final PreparedStatement ps = dataSource.getConnection().prepareStatement(""" + INSERT INTO "VULNERABILITY" ("CVSSV2VECTOR", "CVSSV3VECTOR", "OWASPRRVECTOR", "SOURCE", "UUID", "VULNID") + VALUES (?, ?, ?, ?, ?, ?) + """)) { + for (int i = 0; i < 550; i++) { + ps.setString(1, i % 2 == 0 ? "(AV:N/AC:M/Au:S/C:P/I:P/A:P)" : null); + ps.setString(2, i % 3 == 0 ? "CVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:U/C:H/I:H/A:H" : null); + ps.setString(3, i % 5 == 0 ? "(SL:5/M:5/O:2/S:9/ED:4/EE:2/A:7/ID:2/LC:2/LI:2/LAV:7/LAC:9/FD:3/RD:5/NC:0/PV:7)" : null); + ps.setString(4, "NVD"); + ps.setString(5, UUID.randomUUID().toString()); + ps.setString(6, "CVE-" + i); + ps.addBatch(); + } + + ps.executeBatch(); + } + + assertThat(hasVulnsWithoutSeverity(dataSource.getConnection())).isTrue(); + + Scope.child(Collections.emptyMap(), () -> { + final Database database = DatabaseFactory.getInstance().findCorrectDatabaseImplementation(new JdbcConnection(dataSource.getConnection())); + final var liquibase = new Liquibase("migration/custom/ComputeSeveritiesChangeTest-changelog.xml", new ClassLoaderResourceAccessor(), database); + + final var updateCommand = new CommandScope(UpdateCommandStep.COMMAND_NAME); + updateCommand.addArgumentValue(DbUrlConnectionCommandStep.DATABASE_ARG, liquibase.getDatabase()); + updateCommand.addArgumentValue(UpdateCommandStep.CHANGELOG_FILE_ARG, liquibase.getChangeLogFile()); + updateCommand.execute(); + }); + + assertThat(hasVulnsWithoutSeverity(dataSource.getConnection())).isFalse(); + } + + private boolean hasVulnsWithoutSeverity(final Connection connection) throws Exception { + try (final PreparedStatement ps = connection.prepareStatement(""" + SELECT 1 + FROM "VULNERABILITY" + WHERE "VULNERABILITY"."SEVERITY" IS NULL + """)) { + return ps.executeQuery().next(); + } + } + +} \ No newline at end of file diff --git a/src/test/resources/migration/custom/ComputeSeveritiesChangeTest-changelog.xml b/src/test/resources/migration/custom/ComputeSeveritiesChangeTest-changelog.xml new file mode 100644 index 000000000..39166dade --- /dev/null +++ b/src/test/resources/migration/custom/ComputeSeveritiesChangeTest-changelog.xml @@ -0,0 +1,14 @@ + + + + + + \ No newline at end of file diff --git a/src/test/resources/migration/custom/ComputeSeveritiesChangeTest-schema.sql b/src/test/resources/migration/custom/ComputeSeveritiesChangeTest-schema.sql new file mode 100644 index 000000000..6bb16834f --- /dev/null +++ b/src/test/resources/migration/custom/ComputeSeveritiesChangeTest-schema.sql @@ -0,0 +1,17 @@ +CREATE TABLE "VULNERABILITY" ("ID" BIGINT GENERATED BY DEFAULT AS IDENTITY NOT NULL, "CREATED" TIMESTAMP WITH TIME ZONE, "CREDITS" TEXT, "CVSSV2BASESCORE" numeric, "CVSSV2EXPLOITSCORE" numeric, "CVSSV2IMPACTSCORE" numeric, "CVSSV2VECTOR" VARCHAR(255), "CVSSV3BASESCORE" numeric, "CVSSV3EXPLOITSCORE" numeric, "CVSSV3IMPACTSCORE" numeric, "CVSSV3VECTOR" VARCHAR(255), "CWES" VARCHAR(255), "DESCRIPTION" TEXT, "DETAIL" TEXT, "EPSSPERCENTILE" numeric, "EPSSSCORE" numeric, "FRIENDLYVULNID" VARCHAR(255), "OWASPRRBUSINESSIMPACTSCORE" numeric, "OWASPRRLIKELIHOODSCORE" numeric, "OWASPRRTECHNICALIMPACTSCORE" numeric, "OWASPRRVECTOR" VARCHAR(255), "PATCHEDVERSIONS" VARCHAR(255), "PUBLISHED" TIMESTAMP WITH TIME ZONE, "RECOMMENDATION" TEXT, "REFERENCES" TEXT, "SEVERITY" VARCHAR(255), "SOURCE" VARCHAR(255) NOT NULL, "SUBTITLE" VARCHAR(255), "TITLE" VARCHAR(255), "UPDATED" TIMESTAMP WITH TIME ZONE, "UUID" VARCHAR(36) NOT NULL, "VULNID" VARCHAR(255) NOT NULL, "VULNERABLEVERSIONS" VARCHAR(255), CONSTRAINT "VULNERABILITY_PK" PRIMARY KEY ("ID")); + +CREATE INDEX "VULNERABILITY_VULNID_IDX" ON "VULNERABILITY"("VULNID"); + +CREATE INDEX "VULNERABILITY_PUBLISHED_IDX" ON "VULNERABILITY"("PUBLISHED"); + +CREATE INDEX "VULNERABILITY_UPDATED_IDX" ON "VULNERABILITY"("UPDATED"); + +CREATE INDEX "VULNERABILITY_CREATED_IDX" ON "VULNERABILITY"("CREATED"); + +ALTER TABLE "VULNERABILITY" ADD CONSTRAINT "VULNERABILITY_U1" UNIQUE ("VULNID", "SOURCE"); + +ALTER TABLE "VULNERABILITY" ADD CONSTRAINT "VULNERABILITY_UUID_IDX" UNIQUE ("UUID"); + +ALTER TABLE "VULNERABILITY" DROP COLUMN "EPSSSCORE"; + +ALTER TABLE "VULNERABILITY" DROP COLUMN "EPSSPERCENTILE"; \ No newline at end of file