diff --git a/advisor/build.gradle.kts b/advisor/build.gradle.kts index 6c75e86c9a4c2..afd94cd04e0ee 100644 --- a/advisor/build.gradle.kts +++ b/advisor/build.gradle.kts @@ -29,6 +29,7 @@ dependencies { implementation(projects.utils.ortUtils) + implementation(libs.aeSecurity) implementation(libs.kotlinx.coroutines) testImplementation(libs.mockk) diff --git a/advisor/src/main/kotlin/Advisor.kt b/advisor/src/main/kotlin/Advisor.kt index 57f2bccd1df8a..b63a6eefdcbef 100644 --- a/advisor/src/main/kotlin/Advisor.kt +++ b/advisor/src/main/kotlin/Advisor.kt @@ -27,12 +27,16 @@ import kotlinx.coroutines.withContext import org.apache.logging.log4j.kotlin.logger +import org.metaeffekt.core.security.cvss.CvssVector + import org.ossreviewtoolkit.model.AdvisorResult import org.ossreviewtoolkit.model.AdvisorRun import org.ossreviewtoolkit.model.Identifier import org.ossreviewtoolkit.model.OrtResult import org.ossreviewtoolkit.model.Package import org.ossreviewtoolkit.model.config.AdvisorConfiguration +import org.ossreviewtoolkit.model.vulnerabilities.Vulnerability +import org.ossreviewtoolkit.model.vulnerabilities.VulnerabilityReference import org.ossreviewtoolkit.plugins.api.PluginConfig import org.ossreviewtoolkit.utils.ort.Environment @@ -101,7 +105,9 @@ class Advisor( // Merge results from different providers into a single map keyed by the package ID. The original // provider is still maintained as part of the AdvisorResult's AdvisorDetails. providerResults.await().forEach { (pkg, advisorResults) -> - results.merge(pkg.id, listOf(advisorResults)) { existingResults, additionalResults -> + val normalizedResults = advisorResults.normalizeVulnerabilityData() + + results.merge(pkg.id, listOf(normalizedResults)) { existingResults, additionalResults -> existingResults + additionalResults } } @@ -113,3 +119,37 @@ class Advisor( AdvisorRun(startTime, endTime, Environment(), config, results) } } + +fun AdvisorResult.normalizeVulnerabilityData(): AdvisorResult = + copy(vulnerabilities = vulnerabilities.normalizeVulnerabilityData()) + +fun List.normalizeVulnerabilityData(): List = + map { vulnerability -> + val normalizedReferences = vulnerability.references.map { reference -> + reference + .run { + // Treat "MODERATE" as an alias for "MEDIUM" independently of the scoring system. + if (severity == "MODERATE") copy(severity = "MEDIUM") else this + } + .run { + // Reconstruct the base score from the vector if possible. + if (score == null && vector != null) { + val score = CvssVector.parseVector(vector)?.baseScore?.toFloat() + copy(score = score) + } else { + this + } + } + .run { + // Reconstruct the severity from the scoring system and score if possible. + if (severity == null && scoringSystem != null && score != null) { + val severity = VulnerabilityReference.getQualitativeRating(scoringSystem, score)?.name + copy(severity = severity) + } else { + this + } + } + } + + vulnerability.copy(references = normalizedReferences) + } diff --git a/advisor/src/test/kotlin/AdvisorTest.kt b/advisor/src/test/kotlin/AdvisorTest.kt index 370db5c545a1e..c61010df2aa86 100644 --- a/advisor/src/test/kotlin/AdvisorTest.kt +++ b/advisor/src/test/kotlin/AdvisorTest.kt @@ -154,4 +154,5 @@ private fun mockkAdviceProvider(): AdviceProvider = private fun mockkAdvisorResult(): AdvisorResult = mockk().apply { every { vulnerabilities } returns emptyList() + every { copy(vulnerabilities = any()) } returns this } diff --git a/plugins/advisors/osv/build.gradle.kts b/plugins/advisors/osv/build.gradle.kts index f2860f558a5b9..1f44ee9292bc2 100644 --- a/plugins/advisors/osv/build.gradle.kts +++ b/plugins/advisors/osv/build.gradle.kts @@ -30,7 +30,6 @@ dependencies { implementation(projects.utils.commonUtils) implementation(projects.utils.ortUtils) - implementation(libs.aeSecurity) implementation(libs.kotlinx.serialization.core) implementation(libs.kotlinx.serialization.json) diff --git a/plugins/advisors/osv/src/funTest/assets/retrieve-package-findings-expected-result.json b/plugins/advisors/osv/src/funTest/assets/retrieve-package-findings-expected-result.json index 222c6cd8a37f2..f80fdf269a0f0 100644 --- a/plugins/advisors/osv/src/funTest/assets/retrieve-package-findings-expected-result.json +++ b/plugins/advisors/osv/src/funTest/assets/retrieve-package-findings-expected-result.json @@ -15,25 +15,25 @@ "references" : [ { "url" : "https://nvd.nist.gov/vuln/detail/CVE-2020-7764", "scoring_system" : "CVSS_V3", - "severity" : "MODERATE", + "severity" : "MEDIUM", "score" : 5.9, "vector" : "CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:N/I:N/A:H" }, { "url" : "https://github.com/delvedor/find-my-way/commit/ab408354690e6b9cf3c4724befb3b3fa4bb90aac", "scoring_system" : "CVSS_V3", - "severity" : "MODERATE", + "severity" : "MEDIUM", "score" : 5.9, "vector" : "CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:N/I:N/A:H" }, { "url" : "https://snyk.io/vuln/SNYK-JS-FINDMYWAY-1038269", "scoring_system" : "CVSS_V3", - "severity" : "MODERATE", + "severity" : "MEDIUM", "score" : 5.9, "vector" : "CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:N/I:N/A:H" }, { "url" : "https://www.npmjs.com/package/find-my-way", "scoring_system" : "CVSS_V3", - "severity" : "MODERATE", + "severity" : "MEDIUM", "score" : 5.9, "vector" : "CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:N/I:N/A:H" } ] diff --git a/plugins/advisors/osv/src/funTest/kotlin/OsvFunTest.kt b/plugins/advisors/osv/src/funTest/kotlin/OsvFunTest.kt index 4a87b71dd6b23..8001467eb7b7a 100644 --- a/plugins/advisors/osv/src/funTest/kotlin/OsvFunTest.kt +++ b/plugins/advisors/osv/src/funTest/kotlin/OsvFunTest.kt @@ -28,6 +28,7 @@ import io.kotest.matchers.shouldNot import java.time.Instant +import org.ossreviewtoolkit.advisor.normalizeVulnerabilityData import org.ossreviewtoolkit.model.AdvisorResult import org.ossreviewtoolkit.model.Identifier import org.ossreviewtoolkit.model.Package @@ -94,7 +95,7 @@ private fun createOsv(): Osv = OsvFactory.create() private fun Map.patchTimes(): Map = mapValues { (_, advisorResult) -> - advisorResult.copy( + advisorResult.normalizeVulnerabilityData().copy( summary = advisorResult.summary.copy( startTime = Instant.EPOCH, endTime = Instant.EPOCH diff --git a/plugins/advisors/osv/src/main/kotlin/Osv.kt b/plugins/advisors/osv/src/main/kotlin/Osv.kt index c0fd0b1678276..fbd8702072a06 100644 --- a/plugins/advisors/osv/src/main/kotlin/Osv.kt +++ b/plugins/advisors/osv/src/main/kotlin/Osv.kt @@ -26,8 +26,6 @@ import kotlinx.serialization.json.contentOrNull import org.apache.logging.log4j.kotlin.logger -import org.metaeffekt.core.security.cvss.CvssVector - import org.ossreviewtoolkit.advisor.AdviceProvider import org.ossreviewtoolkit.advisor.AdviceProviderFactory import org.ossreviewtoolkit.clients.osv.OsvServiceWrapper @@ -167,17 +165,10 @@ private fun Vulnerability.toOrtVulnerability(): org.ossreviewtoolkit.model.vulne // Use the 'severity' property of the unspecified 'databaseSpecific' object. // See also https://github.com/google/osv.dev/issues/484. val specificSeverity = databaseSpecific?.get("severity") - - val baseScore = runCatching { - CvssVector.parseVector(vector)?.baseScore?.toFloat() - }.onFailure { - logger.debug { "Unable to parse CVSS vector '$vector': ${it.collectMessages()}." } - }.getOrNull() - val severityRating = (specificSeverity as? JsonPrimitive)?.contentOrNull - ?: VulnerabilityReference.getQualitativeRating(scoringSystem, baseScore)?.name - VulnerabilityReference(it, scoringSystem, severityRating, baseScore, vector) + // OSV never provides the numeric base score as it can be calculated from the vector string. + VulnerabilityReference(it, scoringSystem, severityRating, null, vector) }.getOrNull() } } diff --git a/plugins/advisors/vulnerable-code/src/funTest/kotlin/VulnerableCodeFunTest.kt b/plugins/advisors/vulnerable-code/src/funTest/kotlin/VulnerableCodeFunTest.kt index e1ef63ca18a00..f6c76d052636c 100644 --- a/plugins/advisors/vulnerable-code/src/funTest/kotlin/VulnerableCodeFunTest.kt +++ b/plugins/advisors/vulnerable-code/src/funTest/kotlin/VulnerableCodeFunTest.kt @@ -26,6 +26,7 @@ import io.kotest.matchers.nulls.shouldNotBeNull import io.kotest.matchers.should import io.kotest.matchers.shouldBe +import org.ossreviewtoolkit.advisor.normalizeVulnerabilityData import org.ossreviewtoolkit.model.Identifier import org.ossreviewtoolkit.model.Package import org.ossreviewtoolkit.model.utils.toPurl @@ -37,10 +38,10 @@ class VulnerableCodeFunTest : WordSpec({ val id = Identifier("Go::github.com/quic-go/quic-go:0.40.0") val pkg = Package.EMPTY.copy(id, purl = id.toPurl()) - val findings = vc.retrievePackageFindings(setOf(pkg)) + val results = vc.retrievePackageFindings(setOf(pkg)).values.map { it.normalizeVulnerabilityData() } - findings.values.flatMap { it.summary.issues } should beEmpty() - with(findings.values.flatMap { it.vulnerabilities }.associateBy { it.id }) { + results.flatMap { it.summary.issues } should beEmpty() + with(results.flatMap { it.vulnerabilities }.associateBy { it.id }) { keys shouldContainAll setOf( "CVE-2023-49295" ) @@ -63,10 +64,10 @@ class VulnerableCodeFunTest : WordSpec({ val id = Identifier("Maven:com.google.guava:guava:19.0") val pkg = Package.EMPTY.copy(id, purl = id.toPurl()) - val findings = vc.retrievePackageFindings(setOf(pkg)) + val results = vc.retrievePackageFindings(setOf(pkg)).values.map { it.normalizeVulnerabilityData() } - findings.values.flatMap { it.summary.issues } should beEmpty() - with(findings.values.flatMap { it.vulnerabilities }.associateBy { it.id }) { + results.flatMap { it.summary.issues } should beEmpty() + with(results.flatMap { it.vulnerabilities }.associateBy { it.id }) { keys shouldContainAll setOf( "CVE-2018-10237", "CVE-2020-8908", @@ -89,10 +90,10 @@ class VulnerableCodeFunTest : WordSpec({ val id = Identifier("Maven:org.apache.commons:commons-compress:1.23.0") val pkg = Package.EMPTY.copy(id, purl = id.toPurl()) - val findings = vc.retrievePackageFindings(setOf(pkg)) + val results = vc.retrievePackageFindings(setOf(pkg)).values.map { it.normalizeVulnerabilityData() } - findings.values.flatMap { it.summary.issues } should beEmpty() - with(findings.values.flatMap { it.vulnerabilities }.associateBy { it.id }) { + results.flatMap { it.summary.issues } should beEmpty() + with(results.flatMap { it.vulnerabilities }.associateBy { it.id }) { keys shouldContainAll setOf( "CVE-2023-42503" ) @@ -115,10 +116,10 @@ class VulnerableCodeFunTest : WordSpec({ val id = Identifier("NPM::elliptic:6.5.7") val pkg = Package.EMPTY.copy(id, purl = id.toPurl()) - val findings = vc.retrievePackageFindings(setOf(pkg)) + val results = vc.retrievePackageFindings(setOf(pkg)).values.map { it.normalizeVulnerabilityData() } - findings.values.flatMap { it.summary.issues } should beEmpty() - with(findings.values.flatMap { it.vulnerabilities }.associateBy { it.id }) { + results.flatMap { it.summary.issues } should beEmpty() + with(results.flatMap { it.vulnerabilities }.associateBy { it.id }) { keys shouldContainAll setOf( "CVE-2024-48948" ) diff --git a/plugins/advisors/vulnerable-code/src/main/kotlin/VulnerableCode.kt b/plugins/advisors/vulnerable-code/src/main/kotlin/VulnerableCode.kt index 79b2b09a762c5..3fd072c0145cb 100644 --- a/plugins/advisors/vulnerable-code/src/main/kotlin/VulnerableCode.kt +++ b/plugins/advisors/vulnerable-code/src/main/kotlin/VulnerableCode.kt @@ -155,18 +155,14 @@ class VulnerableCode(override val descriptor: PluginDescriptor, config: Vulnerab if (scores.isEmpty()) return listOf(VulnerabilityReference(sourceUri, null, null, null, null)) - return scores.map { (scoringSystem, scoringElements, value) -> - val score = value.toFloatOrNull() - - val severity = if (score != null) { - VulnerabilityReference.getQualitativeRating(scoringSystem, score)?.name - } else { - // VulnerableCode returns "MODERATE" instead of "MEDIUM" in case of the "cvssv3.1_qr" scoring - // system, see https://github.com/aboutcode-org/vulnerablecode/issues/1186. - value.takeUnless { it == "MODERATE" && scoringSystem == "cvssv3.1_qr" } ?: "MEDIUM" - } + return scores.map { + // In VulnerableCode's data model, a Score class's value is either a numeric score or a severity string. + val score = it.value.toFloatOrNull() + val severity = it.value.takeUnless { score != null } + + val vector = it.scoringElements?.ifEmpty { null } - VulnerabilityReference(sourceUri, scoringSystem, severity, score, scoringElements?.ifEmpty { null }) + VulnerabilityReference(sourceUri, it.scoringSystem, severity, score, vector) } }.onFailure { issues += createAndLogIssue( diff --git a/plugins/advisors/vulnerable-code/src/test/kotlin/VulnerableCodeTest.kt b/plugins/advisors/vulnerable-code/src/test/kotlin/VulnerableCodeTest.kt index a020e7a52d7a1..078561140308e 100644 --- a/plugins/advisors/vulnerable-code/src/test/kotlin/VulnerableCodeTest.kt +++ b/plugins/advisors/vulnerable-code/src/test/kotlin/VulnerableCodeTest.kt @@ -40,6 +40,7 @@ import io.kotest.matchers.shouldBe import java.io.File import java.net.URI +import org.ossreviewtoolkit.advisor.normalizeVulnerabilityData import org.ossreviewtoolkit.model.AdvisorCapability import org.ossreviewtoolkit.model.AdvisorDetails import org.ossreviewtoolkit.model.Identifier @@ -111,7 +112,7 @@ class VulnerableCodeTest : WordSpec({ ) ) ) - langResult.vulnerabilities should containExactly(expLangVulnerability) + langResult.vulnerabilities.normalizeVulnerabilityData() should containExactly(expLangVulnerability) val strutsResult = result.getValue(idStruts) strutsResult.advisor shouldBe vulnerableCode.details @@ -149,7 +150,8 @@ class VulnerableCodeTest : WordSpec({ ) ) ) - strutsResult.vulnerabilities should containExactlyInAnyOrder(expStrutsVulnerabilities) + strutsResult.vulnerabilities.normalizeVulnerabilityData() should + containExactlyInAnyOrder(expStrutsVulnerabilities) } "extract the CVE ID from an alias" { @@ -178,7 +180,8 @@ class VulnerableCodeTest : WordSpec({ ) ) ) - result.getValue(idJUnit).vulnerabilities should containExactly(expJunitVulnerability) + result.getValue(idJUnit).vulnerabilities.normalizeVulnerabilityData() should + containExactly(expJunitVulnerability) } "extract other official identifiers from aliases" { @@ -214,7 +217,8 @@ class VulnerableCodeTest : WordSpec({ ) ) ) - result.getValue(idLog4j).vulnerabilities should containExactlyInAnyOrder(expLog4jVulnerabilities) + result.getValue(idLog4j).vulnerabilities.normalizeVulnerabilityData() should + containExactlyInAnyOrder(expLog4jVulnerabilities) } "handle a failure response from the server" {