[pull] main from dependency-check:main

core/src/main/java/org/owasp/dependencycheck/data/nvdcve/CveDB.java

@@ -1774,22 +1774,22 @@ public void cleanupDatabase() {
                 PreparedStatement psEcosystem = getPreparedStatement(conn, UPDATE_ECOSYSTEM);
                 PreparedStatement psEcosystem2 = getPreparedStatement(conn, UPDATE_ECOSYSTEM2)) {
             if (psEcosystem != null) {
+                final long ecosystemStart = System.currentTimeMillis();
                 final int count = psEcosystem.executeUpdate();
-                if (count > 0) {
-                    LOGGER.info("Updated the CPE ecosystem on {} NVD records", count);
-                }
+                LOGGER.info("Updated the CPE ecosystem on {} NVD records ({} ms)", count,
+                        System.currentTimeMillis() - ecosystemStart);
             }
             if (psEcosystem2 != null) {
+                final long ecosystemStart = System.currentTimeMillis();
                 final int count = psEcosystem2.executeUpdate();
-                if (count > 0) {
-                    LOGGER.info("Removed the CPE ecosystem on {} NVD records", count);
-                }
+                LOGGER.info("Removed the CPE ecosystem on {} NVD records ({} ms)", count,
+                        System.currentTimeMillis() - ecosystemStart);
             }
             if (psOrphans != null) {
+                final long orphanStart = System.currentTimeMillis();
                 final int count = psOrphans.executeUpdate();
-                if (count > 0) {
-                    LOGGER.info("Cleaned up {} orphaned NVD records", count);
-                }
+                LOGGER.info("Cleaned up {} orphaned NVD records ({} ms)", count,
+                        System.currentTimeMillis() - orphanStart);
             }
             final long millis = System.currentTimeMillis() - start;
             //final long seconds = TimeUnit.MILLISECONDS.toSeconds(millis);

core/src/main/resources/data/dbStatements.properties

@@ -38,7 +38,7 @@ INSERT_PROPERTY=INSERT INTO properties (id, `value`) VALUES (?, ?)
 UPDATE_PROPERTY=UPDATE properties SET `value` = ? WHERE id = ?
 DELETE_PROPERTY=DELETE FROM properties WHERE id = ?
 
-UPDATE_ECOSYSTEM=UPDATE cpeEntry e SET e.ecosystem=(SELECT cpeEcosystemCache.ecosystem FROM cpeEcosystemCache WHERE cpeEcosystemCache.vendor=e.vendor AND cpeEcosystemCache.product=e.product AND e.ecosystem IS NULL AND cpeEcosystemCache.ecosystem<>'MULTIPLE') WHERE e.ecosystem IS NULL;
+UPDATE_ECOSYSTEM=UPDATE cpeEntry e SET e.ecosystem=(SELECT cpeEcosystemCache.ecosystem FROM cpeEcosystemCache WHERE cpeEcosystemCache.vendor=e.vendor AND cpeEcosystemCache.product=e.product AND cpeEcosystemCache.ecosystem<>'MULTIPLE') WHERE e.ecosystem IS NULL AND EXISTS(SELECT 1 FROM cpeEcosystemCache WHERE cpeEcosystemCache.vendor=e.vendor AND cpeEcosystemCache.product=e.product AND cpeEcosystemCache.ecosystem<>'MULTIPLE');
 UPDATE_ECOSYSTEM2=UPDATE cpeEntry e SET e.ecosystem=null WHERE e.ecosystem IS NOT NULL AND EXISTS(SELECT * FROM cpeEcosystemCache WHERE cpeEcosystemCache.vendor=e.vendor AND cpeEcosystemCache.product=e.product AND cpeEcosystemCache.ecosystem='MULTIPLE');
 
 SELECT_SCHEMA_VERSION=SELECT value FROM properties WHERE id = 'version'

core/src/main/resources/data/dbStatements_h2.properties

@@ -14,7 +14,7 @@
 
 MERGE_PROPERTY=MERGE INTO properties (id, `value`) KEY(id) VALUES(?, ?)
 MERGE_CPE_ECOSYSTEM=MERGE INTO cpeEcosystemCache (vendor, product, ecosystem) KEY(vendor, product) VALUES(?, ?, ?)
-CLEANUP_ORPHANS=DELETE FROM cpeEntry WHERE id IN (SELECT id FROM cpeEntry LEFT JOIN software ON cpeEntry.id = software.CPEEntryId WHERE software.CPEEntryId IS NULL)
+CLEANUP_ORPHANS=DELETE FROM cpeEntry WHERE NOT EXISTS (SELECT 1 FROM software WHERE software.CPEEntryId = cpeEntry.id)
 
 UPDATE_VULNERABILITY=SELECT * FROM update_vulnerability(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
 

maven/src/main/java/org/owasp/dependencycheck/maven/BaseDependencyCheckMojo.java

@@ -2900,33 +2900,45 @@ protected void checkForFailure(Dependency[] dependencies) throws MojoFailureExce
                 final boolean useUnscored = cvssV2 == -1 && cvssV3 == -1 && cvssV4 == -1;
                 final double unscoredCvss = (useUnscored && v.getUnscoredSeverity() != null) ? SeverityUtil.estimateCvssV2(v.getUnscoredSeverity()) : -1;
 
-                if (failBuildOnCVSS <= 0.0
-                        || cvssV2 >= failBuildOnCVSS
-                        || cvssV3 >= failBuildOnCVSS
-                        || cvssV4 >= failBuildOnCVSS
-                        || unscoredCvss >= failBuildOnCVSS
-                ) {
-                    String name = v.getName();
-                    if (cvssV4 >= 0.0) {
-                        name += "(" + cvssV4 + ")";
-                    } else if (cvssV3 >= 0.0) {
-                        name += "(" + cvssV3 + ")";
-                    } else if (cvssV2 >= 0.0) {
-                        name += "(" + cvssV2 + ")";
-                    } else if (unscoredCvss >= 0.0) {
-                        name += "(" + unscoredCvss + ")";
-                    }
-                    if (addName) {
-                        addName = false;
-                        ids.append(NEW_LINE).append(d.getFileName()).append(" (")
-                                .append(Stream.concat(d.getSoftwareIdentifiers().stream(), d.getVulnerableSoftwareIdentifiers().stream())
-                                        .map(Identifier::getValue)
-                                        .collect(Collectors.joining(", ")))
-                                .append("): ")
-                                .append(name);
+                // the score to display is the one that reached the threshold, so it is picked by
+                // the same comparison that decides whether the vulnerability fails the build
+                final double reportedScore;
+                if (failBuildOnCVSS > 0.0) {
+                    if (cvssV4 >= failBuildOnCVSS) {
+                        reportedScore = cvssV4;
+                    } else if (cvssV3 >= failBuildOnCVSS) {
+                        reportedScore = cvssV3;
+                    } else if (cvssV2 >= failBuildOnCVSS) {
+                        reportedScore = cvssV2;
+                    } else if (unscoredCvss >= failBuildOnCVSS) {
+                        reportedScore = unscoredCvss;
                     } else {
-                        ids.append(", ").append(name);
+                        continue;
                     }
+                } else if (cvssV4 >= 0.0) {
+                    reportedScore = cvssV4;
+                } else if (cvssV3 >= 0.0) {
+                    reportedScore = cvssV3;
+                } else if (cvssV2 >= 0.0) {
+                    reportedScore = cvssV2;
+                } else {
+                    reportedScore = unscoredCvss;
+                }
+
+                String name = v.getName();
+                if (reportedScore >= 0.0) {
+                    name += "(" + reportedScore + ")";
+                }
+                if (addName) {
+                    addName = false;
+                    ids.append(NEW_LINE).append(d.getFileName()).append(" (")
+                            .append(Stream.concat(d.getSoftwareIdentifiers().stream(), d.getVulnerableSoftwareIdentifiers().stream())
+                                    .map(Identifier::getValue)
+                                    .collect(Collectors.joining(", ")))
+                            .append("): ")
+                            .append(name);
+                } else {
+                    ids.append(", ").append(name);
                 }
             }
         }

maven/src/test/java/org/owasp/dependencycheck/maven/BaseDependencyCheckMojoTest.java

@@ -17,19 +17,29 @@
  */
 package org.owasp.dependencycheck.maven;
 
+import io.github.jeremylong.openvulnerability.client.nvd.CvssV2;
+import io.github.jeremylong.openvulnerability.client.nvd.CvssV2Data;
+import org.apache.maven.plugin.MojoFailureException;
 import org.apache.maven.project.MavenProject;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.Spy;
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.owasp.dependencycheck.Engine;
+import org.owasp.dependencycheck.dependency.Dependency;
+import org.owasp.dependencycheck.dependency.Vulnerability;
 import org.owasp.dependencycheck.exception.ExceptionCollection;
+import org.owasp.dependencycheck.utils.CvssUtil;
 
 import java.io.File;
+import java.lang.reflect.Field;
 import java.util.Locale;
 
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.doReturn;
 
 /**
@@ -90,6 +100,82 @@ void should_newDependency_get_pom_declared_as_module() {
         assertEquals(expectOutput, output);
     }
 
+    /**
+     * The build is failed when <em>any</em> of the CVSS scores of a vulnerability reaches the
+     * configured threshold, so the score printed in the failure message must be the score that
+     * actually reached it. Reporting the score of the newest CVSS version instead quotes a score
+     * below the threshold that the very same message states.
+     *
+     * See https://github.com/dependency-check/DependencyCheck/issues/5658
+     */
+    @Test
+    void should_report_the_score_that_reached_the_threshold() throws Exception {
+        // CVE-2021-42550 from the issue: CVSSv2 8.5 fails the build at a threshold of 7.0,
+        // while CVSSv3 is only 6.6.
+        final Dependency dependency = dependencyWith("CVE-2021-42550", 8.5, 6.6);
+
+        final MojoFailureException failure = assertThrows(MojoFailureException.class,
+                () -> mojoWithThreshold(7.0f).checkForFailure(new Dependency[]{dependency}));
+
+        assertTrue(failure.getMessage().contains("CVE-2021-42550(8.5)"), failure.getMessage());
+    }
+
+    /**
+     * With no threshold in play (failBuildOnCVSS &lt;= 0 reports every vulnerability) the newest
+     * CVSS version is still the one to show; this guards the behaviour the change must not alter.
+     */
+    @Test
+    void should_report_the_newest_cvss_version_without_a_threshold() throws Exception {
+        final Dependency dependency = dependencyWith("CVE-2021-42550", 8.5, 6.6);
+
+        final MojoFailureException failure = assertThrows(MojoFailureException.class,
+                () -> mojoWithThreshold(0.0f).checkForFailure(new Dependency[]{dependency}));
+
+        assertTrue(failure.getMessage().contains("CVE-2021-42550(6.6)"), failure.getMessage());
+    }
+
+    /**
+     * A vulnerability whose scores all stay below the threshold is not listed at all, so the
+     * build must not be failed for it.
+     */
+    @Test
+    void should_not_fail_the_build_when_nothing_reached_the_threshold() throws Exception {
+        final Dependency dependency = dependencyWith("CVE-2021-42550", 5.0, 6.6);
+
+        assertDoesNotThrow(() -> mojoWithThreshold(9.0f).checkForFailure(new Dependency[]{dependency}));
+    }
+
+    private static Dependency dependencyWith(String cve, double cvssV2Score, double cvssV3Score) {
+        final Vulnerability vulnerability = new Vulnerability(cve);
+        vulnerability.setCvssV2(cvssV2WithScore(cvssV2Score));
+        vulnerability.setCvssV3(CvssUtil.vectorToCvssV3("CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:L/I:L/A:L", cvssV3Score));
+
+        final Dependency dependency = new Dependency(true);
+        dependency.setFileName("logback-core-1.2.3.jar");
+        dependency.addVulnerability(vulnerability);
+        return dependency;
+    }
+
+    /**
+     * Only the base score is read by {@code checkForFailure}; the remaining metrics are left unset
+     * because {@link CvssUtil#vectorToCvssV2(String, Double)} cannot round-trip a bare CVSSv2
+     * vector string.
+     */
+    private static CvssV2 cvssV2WithScore(double baseScore) {
+        final String severity = CvssUtil.cvssV2ScoreToSeverity(baseScore);
+        final CvssV2Data data = new CvssV2Data(CvssV2Data.Version._2_0, null, null, null, null, null, null, null,
+                baseScore, severity, null, null, null, null, null, null, null, null, null, null);
+        return new CvssV2(null, null, data, severity, null, null, null, null, null, null, null);
+    }
+
+    private static BaseDependencyCheckMojo mojoWithThreshold(float threshold) throws Exception {
+        final BaseDependencyCheckMojo mojo = new BaseDependencyCheckMojoImpl();
+        final Field field = BaseDependencyCheckMojo.class.getDeclaredField("failBuildOnCVSS");
+        field.setAccessible(true);
+        field.setFloat(mojo, threshold);
+        return mojo;
+    }
+
     /**
      * Implementation of ODC Mojo for testing.
      */