diff --git a/addOns/retire/CHANGELOG.md b/addOns/retire/CHANGELOG.md index d007710bf00..e50f5e02755 100644 --- a/addOns/retire/CHANGELOG.md +++ b/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). diff --git a/addOns/retire/src/main/java/org/zaproxy/addon/retire/model/Repo.java b/addOns/retire/src/main/java/org/zaproxy/addon/retire/model/Repo.java index 2a37acdcac7..432b9c56e73 100644 --- a/addOns/retire/src/main/java/org/zaproxy/addon/retire/model/Repo.java +++ b/addOns/retire/src/main/java/org/zaproxy/addon/retire/model/Repo.java @@ -70,7 +70,8 @@ public Repo(Path file) throws IOException { } } - static Map createEntries(Reader reader) throws IOException { + // ** Increased visibility for testing purposes only */ + public static Map createEntries(Reader reader) throws IOException { try { return new ObjectMapper() .readValue(reader, new TypeReference>() {}); @@ -288,7 +289,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 +308,11 @@ private static boolean isGoodCandidate(String version) { return isAllZeros != 0; // Not a good value if all zero } + /** For Testing purposes only */ + public Map getEntries() { + return entries; + } + public static class VulnerabilityData { public static final VulnerabilityData EMPTY = new VulnerabilityData(); diff --git a/addOns/retire/src/main/java/org/zaproxy/addon/retire/model/Vulnerability.java b/addOns/retire/src/main/java/org/zaproxy/addon/retire/model/Vulnerability.java index acf0dd01718..518f6939cda 100644 --- a/addOns/retire/src/main/java/org/zaproxy/addon/retire/model/Vulnerability.java +++ b/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 SEVERITY_MAP = + /** Increased visibility for testing purposes only */ + public static final Map SEVERITY_MAP = Map.of("high", Alert.RISK_HIGH, "medium", Alert.RISK_MEDIUM, "low", Alert.RISK_LOW); private String below; diff --git a/addOns/retire/src/test/java/org/zaproxy/addon/retire/RetireScanRuleUnitTest.java b/addOns/retire/src/test/java/org/zaproxy/addon/retire/RetireScanRuleUnitTest.java index 0e139b91a59..d80a2f77d29 100644 --- a/addOns/retire/src/test/java/org/zaproxy/addon/retire/RetireScanRuleUnitTest.java +++ b/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 diff --git a/addOns/retire/src/test/java/org/zaproxy/addon/retire/RetireTestUtils.java b/addOns/retire/src/test/java/org/zaproxy/addon/retire/RetireTestUtils.java new file mode 100644 index 00000000000..25f6301cab4 --- /dev/null +++ b/addOns/retire/src/test/java/org/zaproxy/addon/retire/RetireTestUtils.java @@ -0,0 +1,40 @@ +/* + * Zed Attack Proxy (ZAP) and its related class files. + * + * ZAP is an HTTP/HTTPS proxy for assessing web application security. + * + * Copyright 2024 The ZAP Development Team + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.zaproxy.addon.retire; + +import java.io.IOException; +import java.io.Reader; +import java.io.StringReader; +import java.nio.charset.StandardCharsets; +import org.apache.commons.io.IOUtils; + +public final class RetireTestUtils { + + private RetireTestUtils() { + } + + public static Reader reader(String fileName) throws IOException { + String content; + try (var is = RetireTestUtils.class.getResourceAsStream(fileName)) { + content = IOUtils.toString(is, StandardCharsets.UTF_8); + } + return new StringReader(content); + } +} diff --git a/addOns/retire/src/test/java/org/zaproxy/addon/retire/UpstreamJsRepositoryUnitTest.java b/addOns/retire/src/test/java/org/zaproxy/addon/retire/UpstreamJsRepositoryUnitTest.java index ef5e32e6b2e..8c6465c4af9 100644 --- a/addOns/retire/src/test/java/org/zaproxy/addon/retire/UpstreamJsRepositoryUnitTest.java +++ b/addOns/retire/src/test/java/org/zaproxy/addon/retire/UpstreamJsRepositoryUnitTest.java @@ -19,10 +19,17 @@ */ package org.zaproxy.addon.retire; -import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; +import java.io.IOException; +import java.io.Reader; +import java.util.Locale; +import java.util.Map; import org.junit.jupiter.api.Test; import org.zaproxy.addon.retire.model.Repo; +import org.zaproxy.addon.retire.model.RepoEntry; +import org.zaproxy.addon.retire.model.Vulnerability; /** Tests for upstream {@code jsrepository.json} file. */ class UpstreamJsRepositoryUnitTest { @@ -31,7 +38,29 @@ class UpstreamJsRepositoryUnitTest { void shouldParseUpstreamJsRepository() { // Given String path = "/org/zaproxy/addon/retire/resources/jsrepository.json"; - // When / Then - assertDoesNotThrow(() -> new Repo(path)); + Reader data; + Map entries = null; + // When + try { + data = RetireTestUtils.reader(path); + entries = Repo.createEntries(data); + } catch (IOException e) { + fail(); + } + // Then + entries.values() + .forEach( + entry -> + entry.getVulnerabilities() + .forEach( + vuln -> + assertTrue( + Vulnerability.SEVERITY_MAP + .keySet() + .contains( + vuln.getSeverity() + .toLowerCase( + Locale + .ROOT))))); } } diff --git a/addOns/retire/src/test/java/org/zaproxy/addon/retire/model/RepoUnitTest.java b/addOns/retire/src/test/java/org/zaproxy/addon/retire/model/RepoUnitTest.java index 4854caba7df..4a45512d5e9 100644 --- a/addOns/retire/src/test/java/org/zaproxy/addon/retire/model/RepoUnitTest.java +++ b/addOns/retire/src/test/java/org/zaproxy/addon/retire/model/RepoUnitTest.java @@ -31,19 +31,20 @@ import java.io.IOException; import java.io.Reader; -import java.io.StringReader; -import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Map; -import org.apache.commons.io.IOUtils; import org.junit.jupiter.api.Test; +import org.zaproxy.addon.retire.RetireTestUtils; /** Unit test for {@link Repo}. */ class RepoUnitTest { + + private static final String PATH_PREFIX = "/org/zaproxy/addon/retire/model/samples/"; + @Test void shouldReadEmptyRepo() throws IOException { // Given - Reader data = reader("repo-empty.json"); + Reader data = RetireTestUtils.reader(PATH_PREFIX + "repo-empty.json"); // When Map entries = Repo.createEntries(data); // Then @@ -53,7 +54,7 @@ void shouldReadEmptyRepo() throws IOException { @Test void shouldReadEmptyLib() throws IOException { // Given - Reader data = reader("lib-empty.json"); + Reader data = RetireTestUtils.reader(PATH_PREFIX + "lib-empty.json"); // When Map entries = Repo.createEntries(data); // Then @@ -67,7 +68,7 @@ void shouldReadEmptyLib() throws IOException { @Test void shouldReadLibWithNoVulnerabilities() throws IOException { // Given - Reader data = reader("vulns-empty.json"); + Reader data = RetireTestUtils.reader(PATH_PREFIX + "vulns-empty.json"); // When Map entries = Repo.createEntries(data); // Then @@ -79,7 +80,7 @@ void shouldReadLibWithNoVulnerabilities() throws IOException { @Test void shouldReadLibWithVulnerabilities() throws IOException { // Given - Reader data = reader("vulns-all.json"); + Reader data = RetireTestUtils.reader(PATH_PREFIX + "vulns-all.json"); // When Map entries = Repo.createEntries(data); // Then @@ -96,13 +97,13 @@ 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 void shouldReadLibWithNoExtractors() throws IOException { // Given - Reader data = reader("extractors-empty.json"); + Reader data = RetireTestUtils.reader(PATH_PREFIX + "extractors-empty.json"); // When Map entries = Repo.createEntries(data); // Then @@ -121,7 +122,7 @@ void shouldReadLibWithNoExtractors() throws IOException { @Test void shouldReadLibWithAllExtractors() throws IOException { // Given - Reader data = reader("extractors-all.json"); + Reader data = RetireTestUtils.reader(PATH_PREFIX + "extractors-all.json"); // When Map entries = Repo.createEntries(data); // Then @@ -146,12 +147,4 @@ void shouldReadLibWithAllExtractors() throws IOException { hasEntry("§§version§§", "§§version§§"))); assertThat(extractors.getUri(), contains("uri 1", "uri 2", "[0-9][0-9a-z._\\-]+?")); } - - private static Reader reader(String fileName) throws IOException { - String content; - try (var is = RepoUnitTest.class.getResourceAsStream("samples/" + fileName)) { - content = IOUtils.toString(is, StandardCharsets.UTF_8); - } - return new StringReader(content); - } } diff --git a/addOns/retire/src/test/resources/org/zaproxy/addon/retire/model/samples/vulns-all.json b/addOns/retire/src/test/resources/org/zaproxy/addon/retire/model/samples/vulns-all.json index b72252cb976..4422ee83b78 100644 --- a/addOns/retire/src/test/resources/org/zaproxy/addon/retire/model/samples/vulns-all.json +++ b/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" ],