Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Loading
Loading status checks…
Fix false positives in CPE matching due to ambiguous vendor/product r…
Browse files Browse the repository at this point in the history 
…elations

Ports DependencyTrack/dependency-track#3209 from DT v4.10.0

Signed-off-by: nscuro <[email protected]>
@nscuro
nscuro committed Feb 8, 2024
1 parent aeb5361 commit d928619
Showing 3 changed files with 276 additions and 136 deletions.
4 changes: 2 additions & 2 deletions ...rc/main/java/org/dependencytrack/persistence/repository/VulnerableSoftwareRepository.java
View file
Original file line number Diff line number Diff line change
@@ -71,8 +71,8 @@ public List<VulnerableSoftware> getAllVulnerableSoftware(final String cpePart, f
// | :-- | :------------- | :--------- | :------- |
// | 1 | ANY | ANY | EQUAL |
// | 5 | NA | ANY | SUBSET |
// | 13 | i | ANY | SUPERSET |
// | 15 | m + wild cards | ANY | SUPERSET |
// | 13 | i | ANY | SUBSET |
// | 15 | m + wild cards | ANY | SUBSET |
cpeQueryFilterParts.add("part is not null");
}

104 changes: 48 additions & 56 deletions ...org/dependencytrack/vulnanalyzer/processor/scanner/internal/InternalScannerProcessor.java
View file
Original file line number Diff line number Diff line change
@@ -10,7 +10,6 @@
import org.apache.kafka.streams.processor.api.ProcessorContext;
import org.apache.kafka.streams.processor.api.Record;
import org.cyclonedx.proto.v1_4.Bom;
import org.dependencytrack.vulnanalyzer.util.ComponentVersion;
import org.dependencytrack.persistence.model.VulnerableSoftware;
import org.dependencytrack.persistence.repository.VulnerableSoftwareRepository;
import org.dependencytrack.proto.vulnanalysis.internal.v1beta1.ScanTask;
@@ -19,15 +18,14 @@
import org.dependencytrack.proto.vulnanalysis.v1.ScanStatus;
import org.dependencytrack.proto.vulnanalysis.v1.Scanner;
import org.dependencytrack.proto.vulnanalysis.v1.ScannerResult;
import org.dependencytrack.vulnanalyzer.util.ComponentVersion;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import us.springett.parsers.cpe.Cpe;
import us.springett.parsers.cpe.CpeParser;
import us.springett.parsers.cpe.exceptions.CpeParsingException;
import us.springett.parsers.cpe.values.LogicalValue;
import us.springett.parsers.cpe.util.Relation;

import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
@@ -78,13 +76,10 @@ private ScannerResult versionRangeAnalysis(final ScanTask task) {

List<VulnerableSoftware> vsList;
String componentVersion;
String componentUpdate;
if (parsedCpe.isPresent()) {
componentVersion = parsedCpe.get().getVersion();
componentUpdate = parsedCpe.get().getUpdate();
} else if (parsedPurl.isPresent()) {
componentVersion = parsedPurl.get().getVersion();
componentUpdate = null;
} else {
// Catch cases where the CPE couldn't be parsed and no PURL exists.
// Should be rare, but could lead to NPEs later.
@@ -117,24 +112,21 @@ private ScannerResult versionRangeAnalysis(final ScanTask task) {
final Cpe cpe = parsedCpe.get();
vsList = QuarkusTransaction.joiningExisting().call(() -> vulnerableSoftwareRepository.getAllVulnerableSoftware(cpe.getPart().getAbbreviation(), cpe.getVendor(), cpe.getProduct(), parsedPurl.orElse(null)));
} else {

vsList = QuarkusTransaction.joiningExisting().call(() -> vulnerableSoftwareRepository.getAllVulnerableSoftware(null, null, null, parsedPurl.orElse(null)));
}

final Bom bov = analyzeVersionRange(vsList, componentVersion, componentUpdate);
final Bom bov = analyzeVersionRange(vsList, parsedCpe.orElse(null), componentVersion);
return resultBuilder
.setStatus(ScanStatus.SCAN_STATUS_SUCCESSFUL)
.setBom(bov)
.build();
}

Bom analyzeVersionRange(final List<VulnerableSoftware> vsList,
final String targetVersion, final String targetUpdate) {
Bom.Builder bov = Bom.newBuilder();
Bom analyzeVersionRange(final List<VulnerableSoftware> vsList, final Cpe targetCpe, final String targetVersion) {
final List<org.cyclonedx.proto.v1_4.Vulnerability> vulnerabilities = new ArrayList<>();
for (final VulnerableSoftware vs : vsList) {
if (compareVersions(vs, targetVersion) && compareUpdate(vs, targetUpdate)) {

final Boolean isCpeMatch = maybeMatchCpe(vs, targetCpe, targetVersion);
if ((isCpeMatch == null || isCpeMatch) && compareVersions(vs, targetVersion)) {
if (vs.getVulnerabilities() != null) {
for (final org.dependencytrack.persistence.model.Vulnerability vulnerability : vs.getVulnerabilities()) {
// Only include vulnerability ID and source in the result. As the vulnerabilities
@@ -148,8 +140,10 @@ Bom analyzeVersionRange(final List<VulnerableSoftware> vsList,
}
}
}
bov.addAllVulnerabilities(vulnerabilities);
return bov.build();

return Bom.newBuilder()
.addAllVulnerabilities(vulnerabilities)
.build();
}

private static Optional<Cpe> parseCpe(final Component component) {
@@ -180,6 +174,43 @@ private static Optional<PackageURL> parsePurl(final Component component) {
}
}

private Boolean maybeMatchCpe(final VulnerableSoftware vs, final Cpe targetCpe, final String targetVersion) {
if (targetCpe == null || vs.getCpe23() == null) {
return null;
}

final List<Relation> relations = List.of(
Cpe.compareAttribute(vs.getPart(), targetCpe.getPart().getAbbreviation()),
Cpe.compareAttribute(vs.getVendor(), targetCpe.getVendor()),
Cpe.compareAttribute(vs.getProduct(), targetCpe.getProduct()),
Cpe.compareAttribute(vs.getVersion(), targetVersion),
Cpe.compareAttribute(vs.getUpdate(), targetCpe.getUpdate()),
Cpe.compareAttribute(vs.getEdition(), targetCpe.getEdition()),
Cpe.compareAttribute(vs.getLanguage(), targetCpe.getLanguage()),
Cpe.compareAttribute(vs.getSwEdition(), targetCpe.getSwEdition()),
Cpe.compareAttribute(vs.getTargetSw(), targetCpe.getTargetSw()),
Cpe.compareAttribute(vs.getTargetHw(), targetCpe.getTargetHw()),
Cpe.compareAttribute(vs.getOther(), targetCpe.getOther())
);
if (relations.contains(Relation.DISJOINT)) {
return false;
}

boolean isMatch = true;

// Mixed SUBSET / SUPERSET relations in the vendor and product attribute are prone
// to false positives: https://github.com/DependencyTrack/dependency-track/issues/3178
final Relation vendorRelation = relations.get(1);
final Relation productRelation = relations.get(2);
isMatch &= !(vendorRelation == Relation.SUBSET && productRelation == Relation.SUPERSET);
isMatch &= !(vendorRelation == Relation.SUPERSET && productRelation == Relation.SUBSET);
if (!isMatch) {
LOGGER.debug("{}: Dropped match with {} due to ambiguous vendor/product relation", targetCpe.toCpe23FS(), vs.getCpe23());
}

return isMatch;
}

static boolean compareVersions(VulnerableSoftware vs, String targetVersion) {
//if any of the four conditions will be evaluated - then true;
boolean result = (vs.getVersionEndExcluding() != null && !vs.getVersionEndExcluding().isEmpty())
@@ -189,7 +220,7 @@ static boolean compareVersions(VulnerableSoftware vs, String targetVersion) {

// Modified from original by Steve Springett
// Added null check: vs.getVersion() != null as purl sources that use version ranges may not have version populated.
if (!result && vs.getVersion() != null && compareAttributes(vs.getVersion(), targetVersion)) {
if (!result && vs.getVersion() != null && Cpe.compareAttribute(vs.getVersion(), targetVersion) != Relation.DISJOINT) {
return true;
}

@@ -216,45 +247,6 @@ static boolean compareVersions(VulnerableSoftware vs, String targetVersion) {
return result;
}

private static final Method COMPARE_ATTRIBUTES_METHOD;

static {
try {
// Workaround for the fact that Cpe#compareAttributes has protected visibility.
COMPARE_ATTRIBUTES_METHOD = Cpe.class.getDeclaredMethod("compareAttributes", String.class, String.class);
COMPARE_ATTRIBUTES_METHOD.setAccessible(true);
} catch (NoSuchMethodException e) {
throw new RuntimeException("Failed to access compareAttributes method of %s".formatted(Cpe.class.getName()), e);
}
}

static boolean compareAttributes(String left, String right) {
try {
return (Boolean) COMPARE_ATTRIBUTES_METHOD.invoke(null, left, right);
} catch (InvocationTargetException | IllegalAccessException e) {
throw new RuntimeException(e);
}
}

static boolean compareUpdate(VulnerableSoftware vs, String targetUpdate) {
if (targetUpdate != null && targetUpdate.equals(vs.getUpdate())) {
return true;
}
if (vs.getUpdate() == null && targetUpdate == null) {
return true;
}
// Moving this above the null OR check to reflect method comments (ANY should mean ANY)
// This is necessary for fuzz matching when a PURL which assumes null
// is matched to a CPE which defaults to ANY
if (LogicalValue.ANY.getAbbreviation().equals(targetUpdate) || LogicalValue.ANY.getAbbreviation().equals(vs.getUpdate())) {
return true;
}
if (vs.getUpdate() == null || targetUpdate == null) {
return false;
}
return compareAttributes(vs.getUpdate(), targetUpdate);
}

private void reportScanResult(final String scanResult) {
componentsScannedCounterBuilder
.tag("result", scanResult)
304 changes: 226 additions & 78 deletions ...dependencytrack/vulnanalyzer/processor/scanner/internal/InternalScannerProcessorTest.java
View file

Large diffs are not rendered by default.

0 comments on commit d928619

Please sign in to comment.