Merge pull request #4208 from nscuro/issue-4175

Fix `affectedComponents` getting removed when updating an internal vulnerability

src/main/java/org/dependencytrack/persistence/QueryManager.java

@@ -915,6 +915,13 @@ public boolean hasAffectedVersionAttribution(final Vulnerability vulnerability,
  return getVulnerabilityQueryManager().hasAffectedVersionAttribution(vulnerability, vulnerableSoftware, source);
  }
 
+ public void synchronizeVulnerableSoftware(
+ final Vulnerability persistentVuln,
+ final List<VulnerableSoftware> vsList,
+ final Vulnerability.Source source) {
+ getVulnerableSoftwareQueryManager().synchronizeVulnerableSoftware(persistentVuln, vsList, source);
+ }
+
  public boolean contains(Vulnerability vulnerability, Component component) {
  return getVulnerabilityQueryManager().contains(vulnerability, component);
  }
@@ -939,6 +946,20 @@ public VulnerableSoftware getVulnerableSoftwareByPurl(String purlType, String pu
  return getVulnerableSoftwareQueryManager().getVulnerableSoftwareByPurl(purlType, purlNamespace, purlName, versionEndExcluding, versionEndIncluding, versionStartExcluding, versionStartIncluding);
  }
 
+ public VulnerableSoftware getVulnerableSoftwareByPurl(
+ final String purl,
+ final String versionEndExcluding,
+ final String versionEndIncluding,
+ final String versionStartExcluding,
+ final String versionStartIncluding) {
+ return getVulnerableSoftwareQueryManager().getVulnerableSoftwareByPurl(
+ purl,
+ versionEndExcluding,
+ versionEndIncluding,
+ versionStartExcluding,
+ versionStartIncluding);
+ }
+
  public List<VulnerableSoftware> getVulnerableSoftwareByVulnId(final String source, final String vulnId) {
  return getVulnerableSoftwareQueryManager().getVulnerableSoftwareByVulnId(source, vulnId);
  }

src/main/java/org/dependencytrack/persistence/VulnerableSoftwareQueryManager.java

@@ -18,22 +18,32 @@
  */
 package org.dependencytrack.persistence;
 
+import alpine.common.logging.Logger;
 import alpine.persistence.PaginatedResult;
 import alpine.resources.AlpineRequest;
 import com.github.packageurl.PackageURL;
+import org.dependencytrack.model.AffectedVersionAttribution;
 import org.dependencytrack.model.Vulnerability;
 import org.dependencytrack.model.VulnerableSoftware;
+import org.dependencytrack.util.PersistenceUtil;
 
 import javax.jdo.PersistenceManager;
 import javax.jdo.Query;
 import java.util.ArrayList;
+import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 
+import static java.util.stream.Collectors.groupingBy;
+import static org.dependencytrack.util.PersistenceUtil.assertNonPersistentAll;
+import static org.dependencytrack.util.PersistenceUtil.assertPersistent;
+
 final class VulnerableSoftwareQueryManager extends QueryManager implements IQueryManager {
 
+ private static final Logger LOGGER = Logger.getLogger(VulnerableSoftwareQueryManager.class);
+
  /**
  * Constructs a new QueryManager.
  * @param pm a PersistenceManager object
@@ -143,6 +153,27 @@ public VulnerableSoftware getVulnerableSoftwareByPurl(String purlType, String pu
  return singleResult(query.executeWithArray(purlType, purlNamespace, purlName, versionEndExcluding, versionEndIncluding, versionStartExcluding, versionStartIncluding));
  }
 
+ /**
+ * @since 4.12.0
+ */
+ public VulnerableSoftware getVulnerableSoftwareByPurl(
+ final String purl,
+ final String versionEndExcluding,
+ final String versionEndIncluding,
+ final String versionStartExcluding,
+ final String versionStartIncluding) {
+ final Query<VulnerableSoftware> query = pm.newQuery(VulnerableSoftware.class);
+ query.setFilter("""
+ purl == :purl \
+ && versionEndExcluding == :versionEndExcluding \
+ && versionEndIncluding == :versionEndIncluding \
+ && versionStartExcluding == :versionStartExcluding \
+ && versionStartIncluding == :versionStartIncluding""");
+ query.setParameters(purl, versionEndExcluding, versionEndIncluding, versionStartExcluding, versionStartIncluding);
+ query.setRange(0, 1);
+ return executeAndCloseUnique(query);
+ }
+
  /**
  * Fetch all {@link VulnerableSoftware} instances associated with a given {@link Vulnerability}.
  *
@@ -316,4 +347,143 @@ public List<VulnerableSoftware> getAllVulnerableSoftware(final String cpePart, f
  }
  }
 
+ /**
+ * @since 4.12.0
+ */
+ public void synchronizeVulnerableSoftware(
+ final Vulnerability persistentVuln,
+ final List<VulnerableSoftware> vsList,
+ final Vulnerability.Source source) {
+ assertPersistent(persistentVuln, "vuln must be persistent");
+ assertNonPersistentAll(vsList, "vsList must not be persistent");
+
+ runInTransaction(() -> {
+ // Get all VulnerableSoftware records that are currently associated with the vulnerability.
+ // Note: For SOME ODD REASON, duplicate (as in, same database ID and all) VulnerableSoftware
+ // records are returned, when operating on data that was originally created by the feed-based
+ // NistMirrorTask. We thus have to deduplicate here.
+ final List<VulnerableSoftware> vsOldList = persistentVuln.getVulnerableSoftware().stream().distinct().toList();
+ LOGGER.trace("%s: Existing VS: %d".formatted(persistentVuln.getVulnId(), vsOldList.size()));
+
+ // Get attributions for all existing VulnerableSoftware records.
+ final Map<Long, List<AffectedVersionAttribution>> attributionsByVsId =
+ getAffectedVersionAttributions(persistentVuln, vsOldList).stream()
+ .collect(groupingBy(attribution -> attribution.getVulnerableSoftware().getId()));
+ for (final VulnerableSoftware vsOld : vsOldList) {
+ vsOld.setAffectedVersionAttributions(attributionsByVsId.get(vsOld.getId()));
+ }
+
+ // Based on the lists of currently reported, and previously reported VulnerableSoftware records,
+ // divide the previously reported ones into lists of records to keep, and records to remove.
+ // Records to keep are removed from vsList. Remaining records in vsList thus are entirely new.
+ final var vsListToRemove = new ArrayList<VulnerableSoftware>();
+ final var vsListToKeep = new ArrayList<VulnerableSoftware>();
+ for (final VulnerableSoftware vsOld : vsOldList) {
+ if (vsList.removeIf(vsOld::equalsIgnoringDatastoreIdentity)) {
+ vsListToKeep.add(vsOld);
+ } else {
+ final List<AffectedVersionAttribution> attributions = vsOld.getAffectedVersionAttributions();
+ if (attributions == null || attributions.isEmpty()) {
+ // DT versions prior to 4.7.0 did not record attributions.
+ // Drop the VulnerableSoftware for now. If it was previously
+ // reported by another source, it will be recorded and attributed
+ // whenever that source is mirrored again.
+ vsListToRemove.add(vsOld);
+ continue;
+ }
+
+ final boolean previouslyReportedBySource = attributions.stream()
+ .anyMatch(attr -> attr.getSource() == source);
+ final boolean previouslyReportedByOthers = !previouslyReportedBySource;
+
+ if (previouslyReportedByOthers) {
+ vsListToKeep.add(vsOld);
+ } else {
+ vsListToRemove.add(vsOld);
+ }
+ }
+ }
+ LOGGER.trace("%s: vsListToKeep: %d".formatted(persistentVuln.getVulnId(), vsListToKeep.size()));
+ LOGGER.trace("%s: vsListToRemove: %d".formatted(persistentVuln.getVulnId(), vsListToRemove.size()));
+
+ // Remove attributions for VulnerableSoftware records that are no longer reported.
+ if (!vsListToRemove.isEmpty()) {
+ deleteAffectedVersionAttributions(persistentVuln, vsListToRemove, source);
+ }
+
+ final var attributionDate = new Date();
+
+ // For VulnerableSoftware records that existed before, update the lastSeen timestamp.
+ for (final VulnerableSoftware oldVs : vsListToKeep) {
+ oldVs.getAffectedVersionAttributions().stream()
+ .filter(attribution -> attribution.getSource() == source)
+ .findAny()
+ .ifPresent(attribution -> attribution.setLastSeen(attributionDate));
+ }
+
+ // For VulnerableSoftware records that are newly reported for this vulnerability, check if any matching
+ // records exist in the database that are currently associated with other (or no) vulnerabilities.
+ for (final VulnerableSoftware vs : vsList) {
+ final VulnerableSoftware existingVs;
+ if (vs.getCpe23() != null) {
+ existingVs = getVulnerableSoftwareByCpe23(
+ vs.getCpe23(),
+ vs.getVersionEndExcluding(),
+ vs.getVersionEndIncluding(),
+ vs.getVersionStartExcluding(),
+ vs.getVersionStartIncluding());
+ } else if (vs.getPurl() != null) {
+ existingVs = getVulnerableSoftwareByPurl(
+ vs.getPurl(),
+ vs.getVersionEndExcluding(),
+ vs.getVersionEndIncluding(),
+ vs.getVersionStartExcluding(),
+ vs.getVersionStartIncluding());
+ } else {
+ throw new IllegalStateException("VulnerableSoftware must define a CPE or PURL, but %s has neither".formatted(vs));
+ }
+ if (existingVs != null) {
+ final boolean hasAttribution = hasAffectedVersionAttribution(persistentVuln, existingVs, source);
+ if (!hasAttribution) {
+ LOGGER.trace("%s: Adding attribution".formatted(persistentVuln.getVulnId()));
+ final AffectedVersionAttribution attribution = createAttribution(persistentVuln, existingVs, attributionDate, source);
+ persist(attribution);
+ } else {
+ LOGGER.debug("%s: Encountered dangling attribution; Re-using by updating firstSeen and lastSeen timestamps".formatted(persistentVuln.getVulnId()));
+ final AffectedVersionAttribution existingAttribution = getAffectedVersionAttribution(persistentVuln, existingVs, source);
+ existingAttribution.setFirstSeen(attributionDate);
+ existingAttribution.setLastSeen(attributionDate);
+ }
+ vsListToKeep.add(existingVs);
+ } else {
+ LOGGER.trace("%s: Creating new VS".formatted(persistentVuln.getVulnId()));
+ final VulnerableSoftware persistentVs = persist(vs);
+ final AffectedVersionAttribution attribution = createAttribution(persistentVuln, persistentVs, attributionDate, source);
+ persist(attribution);
+ vsListToKeep.add(persistentVs);
+ }
+ }
+
+ LOGGER.trace("%s: Final vsList: %d".formatted(persistentVuln.getVulnId(), vsListToKeep.size()));
+ if (!Objects.equals(persistentVuln.getVulnerableSoftware(), vsListToKeep)) {
+ LOGGER.trace("%s: vsList has changed: %s".formatted(persistentVuln.getVulnId(), new PersistenceUtil.Diff(persistentVuln.getVulnerableSoftware(), vsListToKeep)));
+ persistentVuln.setVulnerableSoftware(vsListToKeep);
+ }
+ });
+ }
+
+ private static AffectedVersionAttribution createAttribution(
+ final Vulnerability vuln,
+ final VulnerableSoftware vs,
+ final Date attributionDate,
+ final Vulnerability.Source source) {
+ final var attribution = new AffectedVersionAttribution();
+ attribution.setSource(source);
+ attribution.setVulnerability(vuln);
+ attribution.setVulnerableSoftware(vs);
+ attribution.setFirstSeen(attributionDate);
+ attribution.setLastSeen(attributionDate);
+ return attribution;
+ }
+
 }

src/main/java/org/dependencytrack/resources/v1/VulnerabilityResource.java

@@ -379,12 +379,17 @@ public Response createVulnerability(Vulnerability jsonVulnerability) {
  }
  recalculateScoresAndSeverityFromVectors(jsonVulnerability);
  jsonVulnerability.setSource(Vulnerability.Source.INTERNAL);
- vulnerability = qm.createVulnerability(jsonVulnerability, true);
- qm.persist(vsList);
- qm.updateAffectedVersionAttributions(vulnerability, vsList, Vulnerability.Source.INTERNAL);
- vulnerability.setVulnerableSoftware(vsList);
- qm.persist(vulnerability);
- return Response.status(Response.Status.CREATED).entity(vulnerability).build();
+ return qm.callInTransaction(() -> {
+ final Vulnerability persistentVuln = qm.createVulnerability(jsonVulnerability, true);
+ qm.synchronizeVulnerableSoftware(persistentVuln, vsList, Vulnerability.Source.INTERNAL);
+ if (persistentVuln.getVulnerableSoftware() != null && !persistentVuln.getVulnerableSoftware().isEmpty()) {
+ persistentVuln.setAffectedComponents(persistentVuln.getVulnerableSoftware().stream()
+ .peek(vs -> vs.setAffectedVersionAttributions(qm.getAffectedVersionAttributions(persistentVuln, vs)))
+ .map(AffectedComponent::new)
+ .toList());
+ }
+ return Response.status(Response.Status.CREATED).entity(persistentVuln).build();
+ });
  } else {
  return Response.status(Response.Status.CONFLICT).entity("A vulnerability with the specified vulnId already exists.").build();
  }
@@ -447,7 +452,6 @@ public Response updateVulnerability(Vulnerability jsonVuln) {
  jsonVuln.setCwes(cweIds);
  }
 
- final List<VulnerableSoftware> vsListOld = qm.getVulnerableSoftwareByVulnId(vulnerability.getSource(), vulnerability.getVulnId());
  List<VulnerableSoftware> vsList = new ArrayList<>();
  if (jsonVuln.getAffectedComponents() != null) {
  for (final AffectedComponent ac : jsonVuln.getAffectedComponents()) {
@@ -458,12 +462,17 @@ public Response updateVulnerability(Vulnerability jsonVuln) {
  }
  }
  recalculateScoresAndSeverityFromVectors(jsonVuln);
- vulnerability = qm.updateVulnerability(jsonVuln, true);
- qm.persist(vsList);
- vsList = qm.reconcileVulnerableSoftware(vulnerability, vsListOld, vsList, Vulnerability.Source.INTERNAL);
- vulnerability.setVulnerableSoftware(vsList);
- qm.persist(vulnerability);
- return Response.ok(vulnerability).build();
+ return qm.callInTransaction(() -> {
+ final Vulnerability persistentVuln = qm.updateVulnerability(jsonVuln, true);
+ qm.synchronizeVulnerableSoftware(persistentVuln, vsList, Vulnerability.Source.INTERNAL);
+ if (persistentVuln.getVulnerableSoftware() != null && !persistentVuln.getVulnerableSoftware().isEmpty()) {
+ persistentVuln.setAffectedComponents(persistentVuln.getVulnerableSoftware().stream()
+ .peek(vs -> vs.setAffectedVersionAttributions(qm.getAffectedVersionAttributions(persistentVuln, vs)))
+ .map(AffectedComponent::new)
+ .toList());
+ }
+ return Response.ok(persistentVuln).build();
+ });
  } else {
  return Response.status(Response.Status.NOT_FOUND).entity("The vulnerability could not be found.").build();
  }