Fix internal technical data in aliases of FPF export (DependencyTrack…

…#2471) * fix alias mapping in export Signed-off-by: Lars Meijers <[email protected]> * bumped FPF version in test Signed-off-by: Lars Meijers <[email protected]> --------- Signed-off-by: Lars Meijers <[email protected]> Closes DependencyTrack#2469 Signed-off-by: Anton Soroka <[email protected]>
Hunroll · Mar 1, 2023 · df47d38 · df47d38
1 parent 1c5625a
commit df47d38
Show file tree

Hide file tree

Showing 6 changed files with 128 additions and 7 deletions.
diff --git a/docs/_docs/integrations/file-formats.md b/docs/_docs/integrations/file-formats.md
@@ -30,6 +30,10 @@ The **VIEW_VULNERABILITY** permission is required to use the findings API.
 > Previous versions of Dependency-Track supported only a single CWE (`cweId` and `cweName` fields respectively) per
 > vulnerability.
 > The `cweId` and `cweName` fields are deprecated and will be removed in a later version. Please use `cwes` instead.
+ 
+> Finding Packaging Format v1.2 was introduced in Dependency-Track v4.8.0.
+> It removes the allBySource and the technical 'id' values, which were exposed unintentionally, in the aliases array of a vulnerability.
+> The example below shows how aliases are currently exported.
 
 #### Example
 
@@ -93,6 +97,12 @@ The **VIEW_VULNERABILITY** permission is required to use the findings API.
       "uuid": "701a3953-666b-4b7a-96ca-e1e6a3e1def3",
       "source": "NPM",
       "vulnId": "48",
+      "aliases": [
+        {
+          "cveId": "CVE-2022-2053",
+          "ghsaId": "GHSA-95rf-557x-44g5"
+        }
+      ],
       "title": "Regular Expression Denial of Service",
       "subtitle": "uglify-js",
       "severity": "LOW",

diff --git a/src/main/java/org/dependencytrack/integrations/FindingPackagingFormat.java b/src/main/java/org/dependencytrack/integrations/FindingPackagingFormat.java
@@ -35,7 +35,7 @@
 public class FindingPackagingFormat {
 
     /** FPF is versioned. If the format changes, the version needs to be bumped. */
-    private static final String FPF_VERSION = "1.1";
+    private static final String FPF_VERSION = "1.2";
     private static final String FIELD_APPLICATION = "application";
     private static final String FIELD_VERSION = "version";
     private static final String FIELD_TIMESTAMP = "timestamp";

diff --git a/src/main/java/org/dependencytrack/model/Finding.java b/src/main/java/org/dependencytrack/model/Finding.java
@@ -22,13 +22,18 @@
 import org.apache.commons.lang3.StringUtils;
 import org.dependencytrack.parser.common.resolver.CweResolver;
 import org.dependencytrack.util.VulnerabilityUtil;
+
 import java.io.Serializable;
 import java.math.BigDecimal;
 import java.util.ArrayList;
 import java.util.LinkedHashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.UUID;
+import java.util.Set;
+import java.util.List;
+import java.util.HashSet;
+import java.util.HashMap;
+
 
 /**
  * The Finding object is a metadata/value object that combines data from multiple tables. The object can
@@ -196,4 +201,30 @@ public String getMatrix() {
         return project.toString() + ":" + component.get("uuid") + ":" + vulnerability.get("uuid");
     }
 
+    public void addVulnerabilityAliases(List<VulnerabilityAlias> aliases) {
+        final Set<Map<String, String>> uniqueAliases = new HashSet<>();
+        for (final VulnerabilityAlias alias : aliases) {
+            Map<String,String> map = new HashMap<>();
+            if (alias.getCveId() != null && !alias.getCveId().isBlank()) {
+                map.put("cveId", alias.getCveId());
+            }
+            if (alias.getGhsaId() != null && !alias.getGhsaId().isBlank()) {
+                map.put("ghsaId", alias.getGhsaId());
+            }
+            if (alias.getSonatypeId() != null && !alias.getSonatypeId().isBlank()) {
+                map.put("sonatypeId", alias.getSonatypeId());
+            }
+            if (alias.getOsvId() != null && !alias.getOsvId().isBlank()) {
+                map.put("osvId", alias.getOsvId());
+            }
+            if (alias.getSnykId() != null && !alias.getSnykId().isBlank()) {
+                map.put("snykId", alias.getSnykId());
+            }
+            if (alias.getVulnDbId() != null && !alias.getVulnDbId().isBlank()) {
+                map.put("vulnDbId", alias.getVulnDbId());
+            }
+            uniqueAliases.add(map);
+        }
+        vulnerability.put("aliases",uniqueAliases);
+    }
 }
diff --git a/src/main/java/org/dependencytrack/persistence/FindingsQueryManager.java b/src/main/java/org/dependencytrack/persistence/FindingsQueryManager.java
@@ -275,8 +275,7 @@ public List<Finding> getFindings(Project project, boolean includeSuppressed) {
             final Vulnerability vulnerability = getObjectByUuid(Vulnerability.class, (String)finding.getVulnerability().get("uuid"));
             final Analysis analysis = getAnalysis(component, vulnerability);
             final List<VulnerabilityAlias> aliases = detach(getVulnerabilityAliases(vulnerability));
-            aliases.forEach(alias -> alias.setUuid(null));
-            finding.getVulnerability().put("aliases", aliases);
+            finding.addVulnerabilityAliases(aliases);
             if (includeSuppressed || analysis == null || !analysis.isSuppressed()) { // do not add globally suppressed findings
                 // These are CLOB fields. Handle these here so that database-specific deserialization doesn't need to be performed (in Finding)
                 finding.getVulnerability().put("description", vulnerability.getDescription());

diff --git a/src/test/java/org/dependencytrack/integrations/FindingPackagingFormatTest.java b/src/test/java/org/dependencytrack/integrations/FindingPackagingFormatTest.java
@@ -20,12 +20,19 @@
 
 import alpine.Config;
 import org.dependencytrack.PersistenceCapableTest;
-import org.dependencytrack.model.Project;
+import org.dependencytrack.model.*;
+import org.dependencytrack.tasks.scanners.AnalyzerIdentity;
+import org.json.JSONArray;
 import org.json.JSONObject;
 import org.junit.Assert;
 import org.junit.Test;
 
+import javax.json.JsonArray;
+import java.math.BigDecimal;
 import java.util.Collections;
+import java.util.Date;
+import java.util.List;
+import java.util.UUID;
 
 public class FindingPackagingFormatTest extends PersistenceCapableTest {
 
@@ -50,6 +57,80 @@ public void wrapperTest() {
         Assert.assertEquals(project.getDescription(), pjson.getString("description"));
         Assert.assertEquals(project.getVersion(), pjson.getString("version"));
 
-        Assert.assertEquals("1.1", root.getString("version"));
+        Assert.assertEquals("1.2", root.getString("version"));
+    }
+
+    @Test
+    public void testFindingsVulnerabilityAndAliases() {
+        Project project = qm.createProject(
+                "Test", "Sample project", "1.0", null, null, null, true, false);
+
+        Finding findingWithoutAlias = new Finding(project.getUuid(), "component-uuid-1", "component-name-1", "component-group",
+                "component-version", "component-purl", "component-cpe", "vuln-uuid", Vulnerability.Source.GITHUB, "vuln-vulnId-1", "vuln-title",
+                "vuln-subtitle", "vuln-description", "vuln-recommendation", Severity.CRITICAL, BigDecimal.valueOf(7.2), BigDecimal.valueOf(8.4), BigDecimal.valueOf(1.25), BigDecimal.valueOf(1.75), BigDecimal.valueOf(1.3),
+                "0.5", "0.9", null, AnalyzerIdentity.OSSINDEX_ANALYZER, new Date(), null, null, AnalysisState.NOT_AFFECTED, true);
+
+        Finding findingWithAlias = new Finding(project.getUuid(), "component-uuid-2", "component-name-2", "component-group",
+                "component-version", "component-purl", "component-cpe", "vuln-uuid", Vulnerability.Source.NVD, "vuln-vulnId-2", "vuln-title",
+                "vuln-subtitle", "vuln-description", "vuln-recommendation", Severity.HIGH, BigDecimal.valueOf(7.2), BigDecimal.valueOf(8.4), BigDecimal.valueOf(1.25), BigDecimal.valueOf(1.75), BigDecimal.valueOf(1.3),
+                "0.5", "0.9", null, AnalyzerIdentity.INTERNAL_ANALYZER, new Date(), null, null, AnalysisState.NOT_AFFECTED, true);
+
+        var alias = new VulnerabilityAlias();
+        alias.setCveId("someCveId");
+        alias.setSonatypeId("someSonatypeId");
+        alias.setGhsaId("someGhsaId");
+        alias.setOsvId("someOsvId");
+        alias.setSnykId("someSnykId");
+        alias.setGsdId("someGsdId");
+        alias.setVulnDbId("someVulnDbId");
+        alias.setInternalId("someInternalId");
+
+        var other = new VulnerabilityAlias();
+        other.setCveId("anotherCveId");
+        other.setSonatypeId("anotherSonatypeId");
+        other.setGhsaId("anotherGhsaId");
+        other.setOsvId("anotherOsvId");
+        other.setSnykId("anotherSnykId");
+        other.setGsdId("anotherGsdId");
+        other.setInternalId("anotherInternalId");
+        other.setVulnDbId(null);
+
+        findingWithoutAlias.addVulnerabilityAliases(List.of());
+        findingWithAlias.addVulnerabilityAliases(List.of(alias, other));
+
+        FindingPackagingFormat fpf = new FindingPackagingFormat(
+                project.getUuid(),
+                List.of(findingWithoutAlias, findingWithAlias)
+        );
+
+        JSONObject root = fpf.getDocument();
+
+        JSONArray findings = root.getJSONArray("findings");
+
+        Assert.assertEquals("component-name-1", findings.getJSONObject(0).getJSONObject("component").getString("name"));
+        Assert.assertEquals("component-name-2", findings.getJSONObject(1).getJSONObject("component").getString("name"));
+
+        Assert.assertEquals(AnalyzerIdentity.OSSINDEX_ANALYZER, findings.getJSONObject(0).getJSONObject("attribution").get("analyzerIdentity"));
+        Assert.assertEquals(AnalyzerIdentity.INTERNAL_ANALYZER, findings.getJSONObject(1).getJSONObject("attribution").get("analyzerIdentity"));
+
+        Assert.assertEquals(Severity.CRITICAL.toString(), findings.getJSONObject(0).getJSONObject("vulnerability").get("severity"));
+        Assert.assertEquals(Severity.HIGH.toString(), findings.getJSONObject(1).getJSONObject("vulnerability").get("severity"));
+
+        JSONArray aliases_1 =  findings.getJSONObject(0).getJSONObject("vulnerability").getJSONArray("aliases");
+        Assert.assertTrue(aliases_1.isEmpty());
+        JSONArray aliases_2 =  findings.getJSONObject(1).getJSONObject("vulnerability").getJSONArray("aliases");
+        Assert.assertFalse(aliases_2.isEmpty());
+        Assert.assertEquals(2, aliases_2.length());
+        Assert.assertEquals("anotherCveId", aliases_2.getJSONObject(0).getString("cveId"));
+        Assert.assertEquals("anotherGhsaId", aliases_2.getJSONObject(0).getString("ghsaId"));
+        Assert.assertEquals("someCveId", aliases_2.getJSONObject(1).getString("cveId"));
+        Assert.assertEquals("someOsvId", aliases_2.getJSONObject(1).getString("osvId"));
+
+        // negative test to see if technical id is not included
+        Assert.assertFalse(aliases_2.getJSONObject(0).has("id"));
+
+        //final negative test to make sure the allBySource element is not included
+        String finalJsonOutput = root.toString();
+        Assert.assertFalse(finalJsonOutput.contains("allBySource"));
     }
 }
diff --git a/src/test/java/org/dependencytrack/resources/v1/FindingResourceTest.java b/src/test/java/org/dependencytrack/resources/v1/FindingResourceTest.java
@@ -162,7 +162,7 @@ public void exportFindingsByProjectTest() {
         Assert.assertEquals("Acme Example", json.getJsonObject("project").getString("name"));
         Assert.assertEquals("1.0", json.getJsonObject("project").getString("version"));
         Assert.assertEquals(p1.getUuid().toString(), json.getJsonObject("project").getString("uuid"));
-        Assert.assertEquals("1.1", json.getString("version")); // FPF version
+        Assert.assertEquals("1.2", json.getString("version")); // FPF version
         JsonArray findings = json.getJsonArray("findings");
         Assert.assertEquals(3, findings.size());
         Assert.assertEquals("Component A", findings.getJsonObject(0).getJsonObject("component").getString("name"));