Skip to content

Commit

Permalink
Replaced custom Vers with versatile library (#598)
Browse files Browse the repository at this point in the history
  • Loading branch information
@sahibamittal
sahibamittal authored Mar 22, 2024
1 parent 8ff235b commit cbfb826
Show file tree
Hide file tree
Showing 9 changed files with 122 additions and 370 deletions.
2 changes: 1 addition & 1 deletion pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@
<lib.testcontainers.version>1.19.7</lib.testcontainers.version>
<lib.resilience4j.version>2.2.0</lib.resilience4j.version>
<lib.system-rules.version>1.19.0</lib.system-rules.version>
<lib.versatile.version>0.4.1</lib.versatile.version>
<lib.versatile.version>0.6.0</lib.versatile.version>
<lib.woodstox.version>6.6.0</lib.woodstox.version>
<lib.junit-params.version>1.1.1</lib.junit-params.version>
<lib.log4j-over-slf4j.version>2.0.12</lib.log4j-over-slf4j.version>
Expand Down
159 changes: 85 additions & 74 deletions src/main/java/org/dependencytrack/event/kafka/processor/VulnerabilityMirrorProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
import alpine.common.logging.Logger;
import com.github.packageurl.MalformedPackageURLException;
import com.github.packageurl.PackageURL;
import io.github.nscuro.versatile.Comparator;
import io.github.nscuro.versatile.Constraint;
import io.github.nscuro.versatile.Vers;
import io.github.nscuro.versatile.VersException;
import io.github.nscuro.versatile.version.VersioningScheme;
import org.apache.commons.lang3.StringUtils;
import org.apache.kafka.clients.consumer.ConsumerRecord;
import org.cyclonedx.proto.v1_4.Bom;
Expand All @@ -13,10 +18,6 @@
import org.dependencytrack.model.VulnerableSoftware;
import org.dependencytrack.parser.dependencytrack.ModelConverterCdxToVuln;
import org.dependencytrack.parser.nvd.ModelConverter;
import org.dependencytrack.parser.vers.Comparator;
import org.dependencytrack.parser.vers.Constraint;
import org.dependencytrack.parser.vers.Vers;
import org.dependencytrack.parser.vers.VersException;
import org.dependencytrack.persistence.QueryManager;
import us.springett.parsers.cpe.exceptions.CpeEncodingException;
import us.springett.parsers.cpe.exceptions.CpeParsingException;
Expand Down Expand Up @@ -49,31 +50,31 @@ public void process(final ConsumerRecord<String, Bom> record) {
// Alias synchronization across multiple sources is too unreliable right now.
// We can re-enable this once we have more confidence in data quality, or a better
// way of auditing reported aliases. See also: https://github.com/google/osv.dev/issues/888
// if (!cycloneVuln.getReferencesList().isEmpty()) {
// cycloneVuln.getReferencesList().stream().forEach(reference -> {
// final String alias = reference.getId();
// final VulnerabilityAlias vulnerabilityAlias = new VulnerabilityAlias();
//
// // OSV will use IDs of other vulnerability databases for its
// // primary advisory ID (e.g. GHSA-45hx-wfhj-473x). We need to ensure
// // that we don't falsely report GHSA IDs as stemming from OSV.
// final Vulnerability.Source advisorySource = extractSource(cycloneVuln.getId(), cycloneVuln.getSource());
// if (mirrorSource.equals("OSV")) {
// switch (advisorySource) {
// case NVD -> vulnerabilityAlias.setCveId(cycloneVuln.getId());
// case GITHUB -> vulnerabilityAlias.setGhsaId(cycloneVuln.getId());
// default -> vulnerabilityAlias.setOsvId(cycloneVuln.getId());
// }
// }
// if (alias.startsWith("CVE") && Vulnerability.Source.NVD != advisorySource) {
// vulnerabilityAlias.setCveId(alias);
// qm.synchronizeVulnerabilityAlias(vulnerabilityAlias);
// } else if (alias.startsWith("GHSA") && Vulnerability.Source.GITHUB != advisorySource) {
// vulnerabilityAlias.setGhsaId(alias);
// qm.synchronizeVulnerabilityAlias(vulnerabilityAlias);
// }
// });
// }
/* if (!cycloneVuln.getReferencesList().isEmpty()) {
cycloneVuln.getReferencesList().stream().forEach(reference -> {
final String alias = reference.getId();
final VulnerabilityAlias vulnerabilityAlias = new VulnerabilityAlias();
// OSV will use IDs of other vulnerability databases for its
// primary advisory ID (e.g. GHSA-45hx-wfhj-473x). We need to ensure
// that we don't falsely report GHSA IDs as stemming from OSV.
final Vulnerability.Source advisorySource = extractSource(cycloneVuln.getId(), cycloneVuln.getSource());
if (mirrorSource.equals("OSV")) {
switch (advisorySource) {
case NVD -> vulnerabilityAlias.setCveId(cycloneVuln.getId());
case GITHUB -> vulnerabilityAlias.setGhsaId(cycloneVuln.getId());
default -> vulnerabilityAlias.setOsvId(cycloneVuln.getId());
}
}
if (alias.startsWith("CVE") && Vulnerability.Source.NVD != advisorySource) {
vulnerabilityAlias.setCveId(alias);
qm.synchronizeVulnerabilityAlias(vulnerabilityAlias);
} else if (alias.startsWith("GHSA") && Vulnerability.Source.GITHUB != advisorySource) {
vulnerabilityAlias.setGhsaId(alias);
qm.synchronizeVulnerabilityAlias(vulnerabilityAlias);
}
});
}*/
final List<VulnerableSoftware> vsList = new ArrayList<>();
for (final VulnerabilityAffects affect : cycloneVuln.getAffectsList()) {
final Optional<Component> component = bom.getComponentsList().stream()
Expand All @@ -87,10 +88,10 @@ public void process(final ConsumerRecord<String, Bom> record) {

affect.getVersionsList().forEach(version -> {
if (version.hasRange()) {
final VulnerableSoftware vs = mapAffectedRangeToVulnerableSoftware(qm,
final List<VulnerableSoftware> vs = mapAffectedRangeToVulnerableSoftwares(qm,
vulnerability.getVulnId(), version.getRange(), component.get().getPurl(), component.get().getCpe());
if (vs != null) {
vsList.add(vs);
vsList.addAll(vs);
}
}
if (version.hasVersion()) {
Expand Down Expand Up @@ -159,61 +160,71 @@ public VulnerableSoftware mapAffectedVersionToVulnerableSoftware(final QueryMana
return vs;
}

public VulnerableSoftware mapAffectedRangeToVulnerableSoftware(final QueryManager qm, final String vulnId,
String range, String purlStr, String cpeStr) {
public List<VulnerableSoftware> mapAffectedRangeToVulnerableSoftwares(final QueryManager qm, final String vulnId,
String range, String purlStr, String cpeStr) {
range = StringUtils.trimToNull(range);
cpeStr = StringUtils.trimToNull(cpeStr);
purlStr = StringUtils.trimToNull(purlStr);
if (range == null || (cpeStr == null && purlStr == null)) {
return null;
}

final Vers vers;
final List<VulnerableSoftware> vsList = new ArrayList<>();
final List<Vers> versList;
try {
vers = Vers.parse(range);
versList = convertRangeToVersList(range);
} catch (VersException e) {
LOGGER.warn("Failed to parse vers range from \"%s\" for %s".formatted(range, vulnId), e);
return null;
}

if (vers.constraints().isEmpty()) {
LOGGER.debug("Vers range \"%s\" (parsed: %s) for %s does not contain any constraints; Skipping".formatted(range, vers, vulnId));
return null;
} else if (vers.constraints().size() > 2) {
// Vers ranges can express multiple "branches", which means that this method must potentially be able to return
// multiple `VulnerableSoftware`s, not just one. For example:
// vers:tomee/>=1.0.0-beta1|<=1.7.5|>=7.0.0-M1|<=7.0.7|>=7.1.0|<=7.1.2|>=8.0.0-M1|<=8.0.1
// would result in the following branches:
// * vers:tomee/>=1.0.0-beta1|<=1.7.5
// * vers:tomee/>=7.0.0-M1|<=7.0.7
// * vers:tomee/>=7.1.0|<=7.1.2
// * vers:tomee/>=8.0.0-M1|<=8.0.1
//
// Branches are not always pairs, for example:
// vers:npm/1.2.3|>=2.0.0|<5.0.0
// would result in:
// * vers:npm/1.2.3
// * vers:npm/>=2.0.0|5.0.0
// In both cases, separate `VulnerableSoftware`s are required.
//
// Because mirror-service does not currently produce such complex ranges, we log a warning
// and skip them if we encounter them. Occurrences of this log would indicate broken parsing
// logic in mirror-service.
LOGGER.warn("Vers range \"%s\" (parsed: %s) for %s contains more than two constraints; Skipping".formatted(range, vers, vulnId));
return null;
for (var vers : versList) {
if (vers.constraints().isEmpty()) {
LOGGER.debug("Vers range \"%s\" (parsed: %s) for %s does not contain any constraints; Skipping".formatted(range, vers, vulnId));
continue;
}
else if (vers.constraints().size() == 1) {
var versConstraint = vers.constraints().get(0);
if (versConstraint.comparator() == Comparator.WILDCARD) {
// Wildcards in VulnerableSoftware can be represented via either:
// * version=*, or
// * versionStartIncluding=0
// We choose the more explicit first option.
//
// Also, as wildcards have the potential to lead to lots of false positives,
// we want to be informed when they enter our system. So logging a warning.
LOGGER.warn("Wildcard range %s was reported for %s".formatted(vers, vulnId));
vsList.add(mapAffectedVersionToVulnerableSoftware(qm, vulnId, "*", purlStr, cpeStr));
continue;
}
}
var vulnerableSoftware = convertVersToVulnerableSoftware(qm, vers, vulnId, purlStr, cpeStr);
if (vulnerableSoftware != null) {
vsList.add(vulnerableSoftware);
}
}
return vsList;
}

if (vers.constraints().size() == 1 && vers.constraints().get(0).comparator() == Comparator.WILDCARD) {
// Wildcards in VulnerableSoftware can be represented via either:
// * version=*, or
// * versionStartIncluding=0
// We choose the more explicit first option.
//
// Also, as wildcards have the potential to lead to lots of false positives,
// we want to be informed when they enter our system. So logging a warning.
LOGGER.warn("Wildcard range %s was reported for %s".formatted(vers, vulnId));
return mapAffectedVersionToVulnerableSoftware(qm, vulnId, "*", purlStr, cpeStr);
public static List<Vers> convertRangeToVersList(String range) {
try {
Vers parsedVers = Vers.parse(range);
// Calling split to address ranges with all possible length of constraints
return parsedVers.validate().split();
} catch (VersException versException) {
if (versException.getMessage().contains("invalid versioning scheme")) {
// Fall back the invalid versioning scheme to 'generic' and reparse
String[] rangeParts = range.split(":", 2);
String[] versions = rangeParts[1].split("/", 2);
var genericRange = rangeParts[0] + ":" + VersioningScheme.GENERIC.name().toLowerCase() + "/" + versions[1];
return convertRangeToVersList(genericRange);
} else {
throw versException;
}
}
}

private VulnerableSoftware convertVersToVulnerableSoftware(QueryManager qm, Vers vers, String vulnId, String purlStr, String cpeStr) {

String versionStartIncluding = null;
String versionStartExcluding = null;
Expand All @@ -235,10 +246,10 @@ public VulnerableSoftware mapAffectedRangeToVulnerableSoftware(final QueryManage
}

switch (constraint.comparator()) {
case GREATER_THAN -> versionStartExcluding = constraint.version();
case GREATER_THAN_OR_EQUAL -> versionStartIncluding = constraint.version();
case LESS_THAN_OR_EQUAL -> versionEndIncluding = constraint.version();
case LESS_THAN -> versionEndExcluding = constraint.version();
case GREATER_THAN -> versionStartExcluding = String.valueOf(constraint.version());
case GREATER_THAN_OR_EQUAL -> versionStartIncluding = String.valueOf(constraint.version());
case LESS_THAN_OR_EQUAL -> versionEndIncluding = String.valueOf(constraint.version());
case LESS_THAN -> versionEndExcluding = String.valueOf(constraint.version());
default -> LOGGER.warn("Encountered unexpected comparator %s in %s for %s; Skipping"
.formatted(constraint.comparator(), vers, vulnId));
}
Expand Down
29 changes: 0 additions & 29 deletions src/main/java/org/dependencytrack/parser/vers/Comparator.java
View file

This file was deleted.

68 changes: 0 additions & 68 deletions src/main/java/org/dependencytrack/parser/vers/Constraint.java
View file

This file was deleted.

42 changes: 0 additions & 42 deletions src/main/java/org/dependencytrack/parser/vers/PairwiseIterator.java
View file

This file was deleted.

Loading

0 comments on commit cbfb826

Please sign in to comment.