retire: Fix False Positive issue

- CHANGELOG > Added fix note.
- Repo > Added handling for fall through of essentially "empty" object.
- Tests > Added additional unit tests/assertions.

Signed-off-by: kingthorin <kingthorin@users.noreply.github.com>

addOns/retire/CHANGELOG.md

@@ -4,6 +4,9 @@ All notable changes to this add-on will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## Unreleased
+### Fixed
+- An issue that was resulting in False Positives.
+
 ### Changed
 - Update minimum ZAP version to 2.16.0.
 - The scan rule now uses a more specific CWE (Issue 8732).

addOns/retire/src/main/java/org/zaproxy/addon/retire/model/Repo.java

@@ -288,7 +288,7 @@ private static VulnerabilityData isVersionVulnerable(
                 vulnData.setRisk(vnext.getRisk());
             }
         }
-        return vulnData;
+        return vulnData.getRisk() > 0 ? vulnData : VulnerabilityData.EMPTY;
     }
 
     private static boolean isGoodCandidate(String version) {
@@ -307,6 +307,11 @@ private static boolean isGoodCandidate(String version) {
         return isAllZeros != 0; // Not a good value if all zero
     }
 
+    /** For testing purposes only */
+    Map<String, RepoEntry> getEntries() {
+        return entries;
+    }
+
     public static class VulnerabilityData {
         public static final VulnerabilityData EMPTY = new VulnerabilityData();
 

addOns/retire/src/main/java/org/zaproxy/addon/retire/model/Vulnerability.java

@@ -29,7 +29,8 @@
 @JsonIgnoreProperties({"above", "cwe"})
 public class Vulnerability {
 
-    private static final Map<String, Integer> SEVERITY_MAP =
+    /** Increased visibility for testing purposes only */
+    static final Map<String, Integer> SEVERITY_MAP =
             Map.of("high", Alert.RISK_HIGH, "medium", Alert.RISK_MEDIUM, "low", Alert.RISK_LOW);
 
     private String below;

addOns/retire/src/test/java/org/zaproxy/addon/retire/RetireScanRuleUnitTest.java

@@ -21,6 +21,7 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.is;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertSame;
@@ -183,6 +184,24 @@ void shouldRaiseAlertOnVulnerableContent() {
         assertEquals(4, alertsRaised.get(0).getTags().size());
     }
 
+    @Test
+    void shouldNotRaiseAlertOnNonVulnerableContent() {
+        // Given
+        String content =
+                "/*!\n"
+                        + " * Bootstrap v3.4.0 (http://getbootstrap.com)\n"
+                        + " * Copyright 2011-2016 Twitter, Inc.\n"
+                        + " * Licensed under the MIT license\n"
+                        + " */";
+        HttpMessage msg = createMessage("http://example.com/angular.min.js", content);
+        msg.getResponseHeader().setHeader(HttpFieldsNames.CONTENT_TYPE, "text/javascript");
+        given(passiveScanData.isPage200(any())).willReturn(true);
+        // When
+        scanHttpResponseReceive(msg);
+        // Then
+        assertThat(alertsRaised, hasSize(0));
+    }
+
     @Test
     void shouldRaiseAlertOnHashOfVulnerableContent() {
         // Given

addOns/retire/src/test/java/org/zaproxy/addon/retire/model/RepoUnitTest.java

@@ -96,7 +96,7 @@ void shouldReadLibWithVulnerabilities() throws IOException {
         assertThat(identifiers.getCve(), contains("cve 1", "cve 2"));
         assertThat(identifiers.getSummary(), is("summary"));
         assertThat(vuln.getInfo(), contains("info 1", "info 2"));
-        assertThat(vuln.getSeverity(), is("severity"));
+        assertThat(vuln.getSeverity(), is("high"));
     }
 
     @Test

addOns/retire/src/test/java/org/zaproxy/addon/retire/UpstreamJsRepositoryUnitTest.java → addOns/retire/src/test/java/org/zaproxy/addon/retire/model/UpstreamJsRepositoryUnitTest.java

@@ -17,12 +17,16 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.zaproxy.addon.retire;
+package org.zaproxy.addon.retire.model;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.in;
+import static org.hamcrest.Matchers.is;
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 
+import java.util.Locale;
+import java.util.Set;
 import org.junit.jupiter.api.Test;
-import org.zaproxy.addon.retire.model.Repo;
 
 /** Tests for upstream {@code jsrepository.json} file. */
 class UpstreamJsRepositoryUnitTest {
@@ -31,7 +35,13 @@ class UpstreamJsRepositoryUnitTest {
     void shouldParseUpstreamJsRepository() {
         // Given
         String path = "/org/zaproxy/addon/retire/resources/jsrepository.json";
+        Set<String> expectedSeverities = Vulnerability.SEVERITY_MAP.keySet();
         // When / Then
-        assertDoesNotThrow(() -> new Repo(path));
+        Repo repo = assertDoesNotThrow(() -> new Repo(path));
+        for (RepoEntry entry : repo.getEntries().values()) {
+            for (Vulnerability vuln : entry.getVulnerabilities()) {
+                assertThat(vuln.getSeverity().toLowerCase(Locale.ROOT), is(in(expectedSeverities)));
+            }
+        }
     }
 }

addOns/retire/src/test/resources/org/zaproxy/addon/retire/model/samples/vulns-all.json

@@ -3,7 +3,7 @@
     "vulnerabilities" : [ {
       "atOrAbove" : "atorabove",
       "below" : "below",
-      "severity" : "severity",
+      "severity" : "high",
       "cwe" : [ "cwe 1", "cwe 2" ],
       "identifiers" : {
         "CVE" : [ "cve 1", "cve 2" ],