From 869c9c011412fef2f92acf1dcfb55f22a8250a41 Mon Sep 17 00:00:00 2001 From: Phillip Whittlesea Date: Sun, 11 Jun 2017 12:57:22 +0100 Subject: [PATCH 1/9] Issue #730: Add CLI test for single suppresion file Added @Before and @After for cleaning the singleton Cleaned class to ensure I can add further tests easily I would suggest that AppTest#testPopulateSettings() be split into tests which fail for a single reason. I have avoided that ATM to minimise code I'm meddling with --- .../org/owasp/dependencycheck/AppTest.java | 106 ++++++++++++++---- 1 file changed, 85 insertions(+), 21 deletions(-) diff --git a/dependency-check-cli/src/test/java/org/owasp/dependencycheck/AppTest.java b/dependency-check-cli/src/test/java/org/owasp/dependencycheck/AppTest.java index 9659c5241..06528b0bd 100644 --- a/dependency-check-cli/src/test/java/org/owasp/dependencycheck/AppTest.java +++ b/dependency-check-cli/src/test/java/org/owasp/dependencycheck/AppTest.java @@ -13,28 +13,57 @@ * See the License for the specific language governing permissions and * limitations under the License. * - * Copyright (c) 2017 The OWASP Foundatio. All Rights Reserved. + * Copyright (c) 2017 The OWASP Foundation. All Rights Reserved. */ package org.owasp.dependencycheck; +import static org.hamcrest.core.Is.is; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; + import java.io.File; import java.io.FileNotFoundException; import java.net.URISyntaxException; import java.util.HashMap; import java.util.Map; + import org.apache.commons.cli.ParseException; import org.apache.commons.cli.UnrecognizedOptionException; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; import org.junit.Test; -import static org.junit.Assert.*; +import org.junit.rules.ExpectedException; import org.owasp.dependencycheck.utils.InvalidSettingException; import org.owasp.dependencycheck.utils.Settings; +import org.owasp.dependencycheck.utils.Settings.KEYS; /** - * - * @author jeremy + * Tests for the {@link AppTest} class. */ public class AppTest { + /** Test rule for asserting exceptions and their contents. */ + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + /** + * Initialize the {@link Settings} singleton. + */ + @Before + public void setUp() { + Settings.initialize(); + } + + /** + * Clean the {@link Settings} singleton. + */ + @After + public void tearDown() { + Settings.cleanup(); + } + /** * Test of ensureCanonicalPath method, of class App. */ @@ -52,59 +81,93 @@ public class AppTest { assertTrue("result=" + result, result.endsWith(expResult)); } - @Test(expected = UnrecognizedOptionException.class) - public void testPopulateSettingsException() throws FileNotFoundException, ParseException, InvalidSettingException, URISyntaxException { - String[] args = {"-invalidPROPERTY"}; - assertTrue(testBooleanProperties(args, null)); - } - + /** + * Assert that boolean properties can be set on the CLI and parsed into the {@link Settings} singleton. + * + * @throws Exception the unexpected {@link Exception}. + */ @Test - public void testPopulateSettings() throws FileNotFoundException, ParseException, InvalidSettingException, URISyntaxException { + public void testPopulateSettings() throws Exception { File prop = new File(this.getClass().getClassLoader().getResource("sample.properties").toURI().getPath()); - String[] args = {"-P", prop.getAbsolutePath()}; + String[] args = { "-P", prop.getAbsolutePath() }; Map expected = new HashMap<>(); expected.put(Settings.KEYS.AUTO_UPDATE, Boolean.FALSE); expected.put(Settings.KEYS.ANALYZER_ARCHIVE_ENABLED, Boolean.TRUE); - + assertTrue(testBooleanProperties(args, expected)); - String[] args2 = {"-n"}; + String[] args2 = { "-n" }; expected.put(Settings.KEYS.AUTO_UPDATE, Boolean.FALSE); expected.put(Settings.KEYS.ANALYZER_ARCHIVE_ENABLED, Boolean.TRUE); assertTrue(testBooleanProperties(args2, expected)); - String[] args3 = {"-h"}; + String[] args3 = { "-h" }; expected.put(Settings.KEYS.AUTO_UPDATE, Boolean.TRUE); expected.put(Settings.KEYS.ANALYZER_ARCHIVE_ENABLED, Boolean.TRUE); assertTrue(testBooleanProperties(args3, expected)); - String[] args4 = {"--disableArchive"}; + String[] args4 = { "--disableArchive" }; expected.put(Settings.KEYS.AUTO_UPDATE, Boolean.TRUE); expected.put(Settings.KEYS.ANALYZER_ARCHIVE_ENABLED, Boolean.FALSE); assertTrue(testBooleanProperties(args4, expected)); - String[] args5 = {"-P", prop.getAbsolutePath(), "--disableArchive"}; + String[] args5 = { "-P", prop.getAbsolutePath(), "--disableArchive" }; expected.put(Settings.KEYS.AUTO_UPDATE, Boolean.FALSE); expected.put(Settings.KEYS.ANALYZER_ARCHIVE_ENABLED, Boolean.FALSE); assertTrue(testBooleanProperties(args5, expected)); prop = new File(this.getClass().getClassLoader().getResource("sample2.properties").toURI().getPath()); - String[] args6 = {"-P", prop.getAbsolutePath(), "--disableArchive"}; + String[] args6 = { "-P", prop.getAbsolutePath(), "--disableArchive" }; expected.put(Settings.KEYS.AUTO_UPDATE, Boolean.TRUE); expected.put(Settings.KEYS.ANALYZER_ARCHIVE_ENABLED, Boolean.FALSE); assertTrue(testBooleanProperties(args6, expected)); - String[] args7 = {"-P", prop.getAbsolutePath(), "--noupdate"}; + String[] args7 = { "-P", prop.getAbsolutePath(), "--noupdate" }; expected.put(Settings.KEYS.AUTO_UPDATE, Boolean.FALSE); expected.put(Settings.KEYS.ANALYZER_ARCHIVE_ENABLED, Boolean.FALSE); assertTrue(testBooleanProperties(args7, expected)); - String[] args8 = {"-P", prop.getAbsolutePath(), "--noupdate", "--disableArchive"}; + String[] args8 = { "-P", prop.getAbsolutePath(), "--noupdate", "--disableArchive" }; expected.put(Settings.KEYS.AUTO_UPDATE, Boolean.FALSE); expected.put(Settings.KEYS.ANALYZER_ARCHIVE_ENABLED, Boolean.FALSE); assertTrue(testBooleanProperties(args8, expected)); + } - + /** + * Assert that an {@link UnrecognizedOptionException} is thrown when a property that is not supported is specified on the CLI. + * + * @throws Exception the unexpected {@link Exception}. + */ + @Test + public void testPopulateSettingsException() throws Exception { + String[] args = { "-invalidPROPERTY" }; + + expectedException.expect(UnrecognizedOptionException.class); + expectedException.expectMessage("Unrecognized option: -invalidPROPERTY"); + testBooleanProperties(args, null); + } + + /** + * Assert that a single suppression file can be set using the CLI. + * + * @throws Exception the unexpected {@link Exception}. + */ + @Test + public void testPopulatingSuppressionSettings() throws Exception { + // GIVEN CLI properties with the mandatory arguments + File prop = new File(this.getClass().getClassLoader().getResource("sample.properties").toURI().getPath()); + + // AND a single suppression file + String[] args = { "-P", prop.getAbsolutePath(), "--suppression", "another-file.xml" }; + + // WHEN parsing the CLI arguments + final CliParser cli = new CliParser(); + cli.parse(args); + final App classUnderTest = new App(); + classUnderTest.populateSettings(cli); + + // THEN the suppression file is set in the settings singleton for use in the application core + assertThat("Expected the suppression file to be set in the Settings singleton", Settings.getString(KEYS.SUPPRESSION_FILE), is("another-file.xml")); } private boolean testBooleanProperties(String[] args, Map expected) throws URISyntaxException, FileNotFoundException, ParseException, InvalidSettingException { @@ -124,4 +187,5 @@ public class AppTest { Settings.cleanup(); } } + } From 76218da8d15f21ac6045b44322564ae255c3238a Mon Sep 17 00:00:00 2001 From: Phillip Whittlesea Date: Sun, 11 Jun 2017 15:05:24 +0100 Subject: [PATCH 2/9] Issue #730: Allow multiple args for CLI suppresion The core has not been extended but the CLI is able to parse and pass to the Settings singleton This change to the CLI is backwards compatible --- .../java/org/owasp/dependencycheck/App.java | 5 +- .../org/owasp/dependencycheck/CliParser.java | 14 +-- .../src/site/markdown/arguments.md | 4 +- .../org/owasp/dependencycheck/AppTest.java | 25 ++++- .../owasp/dependencycheck/utils/Settings.java | 30 ++++++ .../dependencycheck/utils/SettingsTest.java | 99 +++++++++++++++++++ 6 files changed, 165 insertions(+), 12 deletions(-) diff --git a/dependency-check-cli/src/main/java/org/owasp/dependencycheck/App.java b/dependency-check-cli/src/main/java/org/owasp/dependencycheck/App.java index 0612c0781..96a111c8d 100644 --- a/dependency-check-cli/src/main/java/org/owasp/dependencycheck/App.java +++ b/dependency-check-cli/src/main/java/org/owasp/dependencycheck/App.java @@ -387,7 +387,7 @@ public class App { final String proxyPass = cli.getProxyPassword(); final String dataDirectory = cli.getDataDirectory(); final File propertiesFile = cli.getPropertiesFile(); - final String suppressionFile = cli.getSuppressionFile(); + final String[] suppressionFiles = cli.getSuppressionFiles(); final String hintsFile = cli.getHintsFile(); final String nexusUrl = cli.getNexusUrl(); final String databaseDriverName = cli.getDatabaseDriverName(); @@ -436,10 +436,11 @@ public class App { Settings.setStringIfNotEmpty(Settings.KEYS.PROXY_USERNAME, proxyUser); Settings.setStringIfNotEmpty(Settings.KEYS.PROXY_PASSWORD, proxyPass); Settings.setStringIfNotEmpty(Settings.KEYS.CONNECTION_TIMEOUT, connectionTimeout); - Settings.setStringIfNotEmpty(Settings.KEYS.SUPPRESSION_FILE, suppressionFile); Settings.setStringIfNotEmpty(Settings.KEYS.HINTS_FILE, hintsFile); Settings.setIntIfNotNull(Settings.KEYS.CVE_CHECK_VALID_FOR_HOURS, cveValidForHours); + Settings.setArrayIfNotEmpty(Settings.KEYS.SUPPRESSION_FILE, suppressionFiles); + //File Type Analyzer Settings Settings.setBooleanIfNotNull(Settings.KEYS.ANALYZER_EXPERIMENTAL_ENABLED, experimentalEnabled); diff --git a/dependency-check-cli/src/main/java/org/owasp/dependencycheck/CliParser.java b/dependency-check-cli/src/main/java/org/owasp/dependencycheck/CliParser.java index 2ac6152c6..e1887e783 100644 --- a/dependency-check-cli/src/main/java/org/owasp/dependencycheck/CliParser.java +++ b/dependency-check-cli/src/main/java/org/owasp/dependencycheck/CliParser.java @@ -273,7 +273,7 @@ public final class CliParser { .desc("Sets how deep nested symbolic links will be followed; 0 indicates symbolic links will not be followed.") .build(); - final Option suppressionFile = Option.builder().argName("file").hasArg().longOpt(ARGUMENT.SUPPRESSION_FILE) + final Option suppressionFile = Option.builder().argName("file").hasArgs().longOpt(ARGUMENT.SUPPRESSION_FILES) .desc("The file path to the suppression XML file.") .build(); @@ -1020,12 +1020,12 @@ public final class CliParser { } /** - * Returns the path to the suppression file. + * Returns the paths to the suppression files. * - * @return the path to the suppression file + * @return the paths to the suppression files. */ - public String getSuppressionFile() { - return line.getOptionValue(ARGUMENT.SUPPRESSION_FILE); + public String[] getSuppressionFiles() { + return line.getOptionValues(ARGUMENT.SUPPRESSION_FILES); } /** @@ -1363,9 +1363,9 @@ public final class CliParser { public static final String SYM_LINK_DEPTH = "symLink"; /** * The CLI argument name for setting the location of the suppression - * file. + * file(s). */ - public static final String SUPPRESSION_FILE = "suppression"; + public static final String SUPPRESSION_FILES = "suppression"; /** * The CLI argument name for setting the location of the hint file. */ diff --git a/dependency-check-cli/src/site/markdown/arguments.md b/dependency-check-cli/src/site/markdown/arguments.md index 048c1f193..d8d1f4376 100644 --- a/dependency-check-cli/src/site/markdown/arguments.md +++ b/dependency-check-cli/src/site/markdown/arguments.md @@ -14,7 +14,7 @@ Short | Argument Name   | Parameter | Description | Requir | \-\-failOnCvss | \ | If the score set between 0 and 10 the exit code from dependency-check will indicate if a vulnerability with a CVSS score equal to or higher was identified. | Optional \-l | \-\-log | \ | The file path to write verbose logging information. | Optional \-n | \-\-noupdate | | Disables the automatic updating of the CPE data. | Optional - | \-\-suppression | \ | The file path to the suppression XML file; used to suppress [false positives](../general/suppression.html). | Optional + | \-\-suppression | \ | The file paths to the suppression XML files; used to suppress [false positives](../general/suppression.html). | Optional \-h | \-\-help | | Print the help message. | Optional | \-\-advancedHelp | | Print the advanced help message. | Optional \-v | \-\-version | | Print the version information. | Optional @@ -64,4 +64,4 @@ Short | Argument Name        | Paramete | \-\-dbPassword | \ | The password for connecting to the database. |   | \-\-dbUser | \ | The username used to connect to the database. |   \-d | \-\-data | \ | The location of the data directory used to store persistent data. This option should generally not be set. |   - | \-\-purge | | Delete the local copy of the NVD. This is used to force a refresh of the data. |   \ No newline at end of file + | \-\-purge | | Delete the local copy of the NVD. This is used to force a refresh of the data. |   diff --git a/dependency-check-cli/src/test/java/org/owasp/dependencycheck/AppTest.java b/dependency-check-cli/src/test/java/org/owasp/dependencycheck/AppTest.java index 06528b0bd..0365bf296 100644 --- a/dependency-check-cli/src/test/java/org/owasp/dependencycheck/AppTest.java +++ b/dependency-check-cli/src/test/java/org/owasp/dependencycheck/AppTest.java @@ -153,7 +153,7 @@ public class AppTest { * @throws Exception the unexpected {@link Exception}. */ @Test - public void testPopulatingSuppressionSettings() throws Exception { + public void testPopulatingSuppressionSettingsWithASingleFile() throws Exception { // GIVEN CLI properties with the mandatory arguments File prop = new File(this.getClass().getClassLoader().getResource("sample.properties").toURI().getPath()); @@ -170,6 +170,29 @@ public class AppTest { assertThat("Expected the suppression file to be set in the Settings singleton", Settings.getString(KEYS.SUPPRESSION_FILE), is("another-file.xml")); } + /** + * Assert that multiple suppression files can be set using the CLI. + * + * @throws Exception the unexpected {@link Exception}. + */ + @Test + public void testPopulatingSuppressionSettingsWithMultipleFiles() throws Exception { + // GIVEN CLI properties with the mandatory arguments + File prop = new File(this.getClass().getClassLoader().getResource("sample.properties").toURI().getPath()); + + // AND a single suppression file + String[] args = { "-P", prop.getAbsolutePath(), "--suppression", "first-file.xml", "another-file.xml" }; + + // WHEN parsing the CLI arguments + final CliParser cli = new CliParser(); + cli.parse(args); + final App classUnderTest = new App(); + classUnderTest.populateSettings(cli); + + // THEN the suppression file is set in the settings singleton for use in the application core + assertThat("Expected the suppression files to be set in the Settings singleton with a separator", Settings.getString(KEYS.SUPPRESSION_FILE), is("first-file.xml,another-file.xml")); + } + private boolean testBooleanProperties(String[] args, Map expected) throws URISyntaxException, FileNotFoundException, ParseException, InvalidSettingException { Settings.initialize(); try { diff --git a/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java b/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java index da96ad1c6..da8b4435b 100644 --- a/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java +++ b/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java @@ -32,6 +32,8 @@ import java.net.URLDecoder; import java.util.Enumeration; import java.util.Properties; +import org.apache.commons.lang3.StringUtils; + /** * A simple settings container that wraps the dependencycheck.properties file. * @@ -47,6 +49,10 @@ public final class Settings { * The properties file location. */ private static final String PROPERTIES_FILE = "dependencycheck.properties"; + /** + * Array separator. + */ + private static final String ARRAY_SEP = ","; /** * Thread local settings. */ @@ -558,6 +564,18 @@ public final class Settings { LOGGER.debug("Setting: {}='{}'", key, value); } + /** + * Sets a property value from an array. + *

+ * Note: each value of the array will be joined by the delimiter {@link Settings#ARRAY_SEP}. + * + * @param key the key for the property + * @param value the value for the property + */ + static void setArray(String key, String[] value) { + setString(key, StringUtils.join(value, ARRAY_SEP)); + } + /** * Sets a property value only if the value is not null. * @@ -582,6 +600,18 @@ public final class Settings { } } + /** + * Sets a property value only if the array value is not null and not empty. + * + * @param key the key for the property + * @param value the value for the property + */ + public static void setArrayIfNotEmpty(String key, String[] value) { + if (null != value && value.length > 0) { + setArray(key, value); + } + } + /** * Sets a property value. * diff --git a/dependency-check-utils/src/test/java/org/owasp/dependencycheck/utils/SettingsTest.java b/dependency-check-utils/src/test/java/org/owasp/dependencycheck/utils/SettingsTest.java index 5d0c96f93..79199cbd7 100644 --- a/dependency-check-utils/src/test/java/org/owasp/dependencycheck/utils/SettingsTest.java +++ b/dependency-check-utils/src/test/java/org/owasp/dependencycheck/utils/SettingsTest.java @@ -17,10 +17,17 @@ */ package org.owasp.dependencycheck.utils; +import static org.hamcrest.core.Is.is; +import static org.hamcrest.core.IsNull.nullValue; +import static org.junit.Assert.assertThat; + import java.io.File; import java.io.IOException; import java.net.URISyntaxException; + +import org.junit.After; import org.junit.Assert; +import org.junit.Before; import org.junit.Test; /** @@ -29,6 +36,22 @@ import org.junit.Test; */ public class SettingsTest extends BaseTest { + /** + * Initialize the {@link Settings} singleton. + */ + @Before + public void setUp() { + Settings.initialize(); + } + + /** + * Clean the {@link Settings} singleton. + */ + @After + public void tearDown() { + Settings.cleanup(); + } + /** * Test of getString method, of class Settings. */ @@ -220,4 +243,80 @@ public class SettingsTest extends BaseTest { File tmp = Settings.getTempDirectory(); Assert.assertTrue(tmp.exists()); } + + /** + * Assert {@link Settings#setArrayIfNotEmpty(String, String[])} with an empty array is ignored. + */ + @Test + public void testSetArrayNotEmptyIgnoresAnEmptyArray() { + // GIVEN an empty array + final String[] array = {}; + + // WHEN setting the array + Settings.setArrayIfNotEmpty("key", array); + + // THEN the property was not set + assertThat("Expected the property to not be set", Settings.getString("key"), nullValue()); + } + + /** + * Assert {@link Settings#setArrayIfNotEmpty(String, String[])} with a null array is ignored. + */ + @Test + public void testSetArrayNotEmptyIgnoresAnNullArray() { + // GIVEN a null array + final String[] array = null; + + // WHEN setting the array + Settings.setArrayIfNotEmpty("key", array); + + // THEN the property was not set + assertThat("Expected the property to not be set", Settings.getString("key"), nullValue()); + } + + /** + * Assert {@link Settings#setArrayIfNotEmpty(String, String[])} with multiple values sets a delimited string. + */ + @Test + public void testSetArrayNotEmptySetsADelimitedString() { + // GIVEN an array with values + final String[] array = { "value1", "value2" }; + + // WHEN setting the array + Settings.setArrayIfNotEmpty("key", array); + + // THEN the property is set + assertThat("Expected the property to be set", Settings.getString("key"), is("value1,value2")); + } + + /** + * Assert {@link Settings#setArrayIfNotEmpty(String, String[])} with a single values sets a string. + */ + @Test + public void testSetArrayNotEmptyWithSingleValueSetsAString() { + // GIVEN an array with a value + final String[] array = { "value1" }; + + // WHEN setting the array + Settings.setArrayIfNotEmpty("key", array); + + // THEN the property is set + assertThat("Expected the property to be set", Settings.getString("key"), is("value1")); + } + + /** + * Assert {@link Settings#setArray(String, String[])} with multiple values sets a delimited string. + */ + @Test + public void testSetArraySetsADelimitedString() { + // GIVEN an array with values + final String[] array = { "value1", "value2" }; + + // WHEN setting the array + Settings.setArray("key", array); + + // THEN the property is set + assertThat("Expected the property to be set", Settings.getString("key"), is("value1,value2")); + } + } From ed214d05fa8829b9ad4ef000636a8db11953a8f5 Mon Sep 17 00:00:00 2001 From: Phillip Whittlesea Date: Sun, 11 Jun 2017 16:06:32 +0100 Subject: [PATCH 3/9] Issue #730: Add a test for suppression in an Ant task --- .gitignore | 1 + .../taskdefs/DependencyCheckTaskTest.java | 14 +++++++++++ .../src/test/resources/build.xml | 23 +++++++++++++++---- .../src/test/resources/test-suppression.xml | 11 +++++++++ 4 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 dependency-check-ant/src/test/resources/test-suppression.xml diff --git a/.gitignore b/.gitignore index 0f79d7db7..e7a69cf2e 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ */target/** # IntelliJ test run side-effects dependency-check-core/data/ +dependency-check-ant/data/ # Intellij project files *.iml *.ipr diff --git a/dependency-check-ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskTest.java b/dependency-check-ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskTest.java index ffab315f3..018794355 100644 --- a/dependency-check-ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskTest.java +++ b/dependency-check-ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskTest.java @@ -114,4 +114,18 @@ public class DependencyCheckTaskTest { expectedException.expect(BuildException.class); buildFileRule.executeTarget("failCVSS"); } + + /** + * Test the DependencyCheckTask where a CVE is suppressed. + */ + @Test + public void testSuppressingCVE() { + // GIVEN an ant task with a vulnerability + final String antTaskName = "suppression"; + + // WHEN executing the ant task + buildFileRule.executeTarget(antTaskName); + + // THEN the ant task executed without error + } } diff --git a/dependency-check-ant/src/test/resources/build.xml b/dependency-check-ant/src/test/resources/build.xml index 11808a5b9..be5856c61 100644 --- a/dependency-check-ant/src/test/resources/build.xml +++ b/dependency-check-ant/src/test/resources/build.xml @@ -61,11 +61,24 @@ + applicationName="test failCVSS" + reportOutputDirectory="${project.build.directory}" + reportFormat="XML" + autoupdate="false" + failBuildOnCVSS="3"> + + + + + + + + diff --git a/dependency-check-ant/src/test/resources/test-suppression.xml b/dependency-check-ant/src/test/resources/test-suppression.xml new file mode 100644 index 000000000..abbc2017d --- /dev/null +++ b/dependency-check-ant/src/test/resources/test-suppression.xml @@ -0,0 +1,11 @@ + + + + + + ^org\.apache\.axis:axis:.*$ + cpe:/a:apache:axis + + From 237dbe706152f4467d1d3f8dc42703514740c25c Mon Sep 17 00:00:00 2001 From: Phillip Whittlesea Date: Sun, 11 Jun 2017 18:45:07 +0100 Subject: [PATCH 4/9] Issue #730: Allow multiple suppression files in Ant The core has not been extended but the Ant task is able to parse and pass to the Settings singleton NOTE: This change is breaking for users of the Ant Task --- .../owasp/dependencycheck/taskdefs/Check.java | 32 +++++++++--- .../taskdefs/SuppressionFile.java | 49 +++++++++++++++++++ .../src/site/markdown/configuration.md | 10 +++- .../taskdefs/DependencyCheckTaskTest.java | 15 ++++++ .../src/test/resources/build.xml | 14 +++++- ...-suppression.xml => test-suppression1.xml} | 0 .../analyzer/AbstractSuppressionAnalyzer.java | 10 ++-- .../owasp/dependencycheck/utils/Settings.java | 16 ++++++ .../dependencycheck/utils/SettingsTest.java | 32 ++++++++++++ 9 files changed, 163 insertions(+), 15 deletions(-) create mode 100644 dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/SuppressionFile.java rename dependency-check-ant/src/test/resources/{test-suppression.xml => test-suppression1.xml} (100%) diff --git a/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java b/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java index 9841a0830..65b6cd9a8 100644 --- a/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java +++ b/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java @@ -18,7 +18,9 @@ package org.owasp.dependencycheck.taskdefs; import java.io.File; +import java.util.ArrayList; import java.util.List; + import org.apache.tools.ant.BuildException; import org.apache.tools.ant.Project; import org.apache.tools.ant.types.EnumeratedAttribute; @@ -148,9 +150,10 @@ public class Check extends Update { */ private String reportFormat = "HTML"; /** - * The path to the suppression file. + * Suppression file paths. */ - private String suppressionFile; + private List suppressionFiles = new ArrayList<>(); + /** * The path to the suppression file. */ @@ -225,6 +228,17 @@ public class Check extends Update { getPath().add(rc); } + /** + * Add a suppression file. + * + * This is called by Ant with the configured {@link SuppressionFile}. + * + * @param suppressionFile the suppression file to add. + */ + public void addConfiguredSuppressionFile(final SuppressionFile suppressionFile) { + suppressionFiles.add(suppressionFile.getPath()); + } + /** * Returns the path. If the path has not been initialized yet, this class is * synchronized, and will instantiate the path object. @@ -431,21 +445,23 @@ public class Check extends Update { } /** - * Get the value of suppressionFile. + * Gets suppression file paths. * - * @return the value of suppressionFile + * @return the suppression files. */ - public String getSuppressionFile() { - return suppressionFile; + public List getSuppressionFiles() { + return suppressionFiles; } /** * Set the value of suppressionFile. * * @param suppressionFile new value of suppressionFile + * @deprecated property form of suppressionFile has been replaced by a child element */ + @Deprecated public void setSuppressionFile(String suppressionFile) { - this.suppressionFile = suppressionFile; + throw new BuildException("Property form of suppressionFile has been replaced by a nested element, please update your configuration."); } /** @@ -992,7 +1008,7 @@ public class Check extends Update { protected void populateSettings() throws BuildException { super.populateSettings(); Settings.setBooleanIfNotNull(Settings.KEYS.AUTO_UPDATE, autoUpdate); - Settings.setStringIfNotEmpty(Settings.KEYS.SUPPRESSION_FILE, suppressionFile); + Settings.setArrayIfNotEmpty(Settings.KEYS.SUPPRESSION_FILE, suppressionFiles.toArray(new String[suppressionFiles.size()])); Settings.setStringIfNotEmpty(Settings.KEYS.HINTS_FILE, hintsFile); Settings.setBooleanIfNotNull(Settings.KEYS.ANALYZER_EXPERIMENTAL_ENABLED, enableExperimental); Settings.setBooleanIfNotNull(Settings.KEYS.ANALYZER_JAR_ENABLED, jarAnalyzerEnabled); diff --git a/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/SuppressionFile.java b/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/SuppressionFile.java new file mode 100644 index 000000000..962c09cad --- /dev/null +++ b/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/SuppressionFile.java @@ -0,0 +1,49 @@ +/* + * This file is part of dependency-check-ant. + * + * 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. + * + * Copyright (c) 2017 The OWASP Foundation. All Rights Reserved. + */ +package org.owasp.dependencycheck.taskdefs; + +/** + * Class : {@link SuppressionFile} + * Responsibility : Models a suppression file nested XML element where the simple content is its location. + */ +public class SuppressionFile { + + /** + * The path to the suppression file. + */ + private String path; + + /** + * Called by ant with the simple content of the suppressionFile xml element. + * + * @param text the simple content. + */ + public final void addText(String text) { + this.path = text; + } + + /** + * Gets the path to the suppression file. + * + * @return the path. + */ + public String getPath() { + return path; + } + +} diff --git a/dependency-check-ant/src/site/markdown/configuration.md b/dependency-check-ant/src/site/markdown/configuration.md index 8b0b84634..680a76cd8 100644 --- a/dependency-check-ant/src/site/markdown/configuration.md +++ b/dependency-check-ant/src/site/markdown/configuration.md @@ -18,6 +18,7 @@ the project's dependencies. reportoutputdirectory="${basedir}" reportformat="ALL"> + path/to/suppression.xml @@ -38,14 +39,19 @@ failOnError | Whether the build should fail if there is an error execu projectName | The name of the project being scanned. | Dependency-Check reportFormat | The report format to be generated (HTML, XML, CSV, JSON, VULN, ALL). This configuration option has no affect if using this within the Site plugin unless the externalReport is set to true. | HTML reportOutputDirectory | The location to write the report(s). Note, this is not used if generating the report as part of a `mvn site` build | 'target' -suppressionFile | The file path to the XML suppression file \- used to suppress [false positives](../general/suppression.html) |   hintsFile | The file path to the XML hints file \- used to resolve [false negatives](../general/hints.html) |   proxyServer | The Proxy Server; see the [proxy configuration](../data/proxy.html) page for more information. |   proxyPort | The Proxy Port. |   proxyUsername | Defines the proxy user name. |   proxyPassword | Defines the proxy password. |   connectionTimeout | The URL Connection Timeout. |   -enableExperimental | Enable the [experimental analyzers](../analyzers/index.html). If not enabled the experimental analyzers (see below) will not be loaded or used. | false +enableExperimental | Enable the [experimental analyzers](../analyzers/index.html). If not enabled the experimental analyzers (see below) will not be loaded or used. | false + +The following nested elements can be set on the dependency-check task. + +Property | Description | Default Value +----------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------------- +suppressionFile | The file path to the XML suppression file \- used to suppress [false positives](../general/suppression.html). Element can be specified multiple times. |   Analyzer Configuration ==================== diff --git a/dependency-check-ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskTest.java b/dependency-check-ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskTest.java index 018794355..87beaf6d9 100644 --- a/dependency-check-ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskTest.java +++ b/dependency-check-ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskTest.java @@ -128,4 +128,19 @@ public class DependencyCheckTaskTest { // THEN the ant task executed without error } + + /** + * Test the DependencyCheckTask deprecated suppression property throws an exception with a warning. + */ + @Test + public void testDeprecatedSuppressingCVE() { + // GIVEN an ant task with a vulnerability using the legacy property + final String antTaskName = "deprecated-suppression"; + + // WHEN executing the ant task + // THEN an exception with a warning is thrown + expectedException.expect(BuildException.class); + expectedException.expectMessage("Property form of suppressionFile has been replaced by a nested element, please update your configuration."); + buildFileRule.executeTarget(antTaskName); + } } diff --git a/dependency-check-ant/src/test/resources/build.xml b/dependency-check-ant/src/test/resources/build.xml index be5856c61..d7a2bf845 100644 --- a/dependency-check-ant/src/test/resources/build.xml +++ b/dependency-check-ant/src/test/resources/build.xml @@ -72,13 +72,23 @@ - + + suppressionFile="${project.build.directory}/test-classes/test-suppression1.xml"/> + + + + + ${project.build.directory}/test-classes/test-suppression1.xml + ${project.build.directory}/test-classes/test-suppression2.xml diff --git a/dependency-check-ant/src/test/resources/test-suppression.xml b/dependency-check-ant/src/test/resources/test-suppression1.xml similarity index 100% rename from dependency-check-ant/src/test/resources/test-suppression.xml rename to dependency-check-ant/src/test/resources/test-suppression1.xml diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java index b5a74171a..2a24a7892 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java @@ -71,7 +71,7 @@ public abstract class AbstractSuppressionAnalyzer extends AbstractAnalyzer { try { loadSuppressionData(); } catch (SuppressionParseException ex) { - throw new InitializationException("Error initializing the suppression analyzer", ex); + throw new InitializationException("Error initializing the suppression analyzer: " + ex.getLocalizedMessage(), ex); } } @@ -112,10 +112,14 @@ public abstract class AbstractSuppressionAnalyzer extends AbstractAnalyzer { } catch (SAXException ex) { throw new SuppressionParseException("Unable to parse the base suppression data file", ex); } - final String suppressionFilePath = Settings.getString(Settings.KEYS.SUPPRESSION_FILE); - if (suppressionFilePath == null) { + final String[] suppressionFilePaths = Settings.getArray(Settings.KEYS.SUPPRESSION_FILE); + if (suppressionFilePaths == null || suppressionFilePaths.length == 0) { return; } + + // TODO support more than one file + final String suppressionFilePath = suppressionFilePaths[0]; + boolean deleteTempFile = false; try { final Pattern uriRx = Pattern.compile("^(https?|file)\\:.*", Pattern.CASE_INSENSITIVE); diff --git a/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java b/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java index da8b4435b..32aa3147b 100644 --- a/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java +++ b/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java @@ -821,6 +821,22 @@ public final class Settings { return System.getProperty(key, LOCAL_SETTINGS.get().props.getProperty(key)); } + /** + * Returns a list with the given key. + * + * If the propery is not set then {@code null} will be returned. + * + * @param key the key to get from this {@link Settings} singleton. + * @return the list or {@code null} if the key wasn't present. + */ + public static String[] getArray(final String key) { + final String string = getString(key); + if (string != null) { + return string.split(ARRAY_SEP); + } + return null; + } + /** * Removes a property from the local properties collection. This is mainly * used in test cases. diff --git a/dependency-check-utils/src/test/java/org/owasp/dependencycheck/utils/SettingsTest.java b/dependency-check-utils/src/test/java/org/owasp/dependencycheck/utils/SettingsTest.java index 79199cbd7..881be4138 100644 --- a/dependency-check-utils/src/test/java/org/owasp/dependencycheck/utils/SettingsTest.java +++ b/dependency-check-utils/src/test/java/org/owasp/dependencycheck/utils/SettingsTest.java @@ -18,6 +18,7 @@ package org.owasp.dependencycheck.utils; import static org.hamcrest.core.Is.is; +import static org.hamcrest.core.IsNull.notNullValue; import static org.hamcrest.core.IsNull.nullValue; import static org.junit.Assert.assertThat; @@ -244,6 +245,37 @@ public class SettingsTest extends BaseTest { Assert.assertTrue(tmp.exists()); } + /** + * Assert {@link Settings#getArray(String)} from a delimited string returns multiple values in an array. + */ + @Test + public void testGetArrayFromADelimitedString() { + // GIVEN a delimited string + final String delimitedString = "value1,value2"; + Settings.setString("key", delimitedString); + + // WHEN getting the array + final String[] array = Settings.getArray("key"); + + // THEN the split array is returned + assertThat("Expected the array to be non-null", array, notNullValue()); + assertThat("Expected the array to have two values", array.length, is(2)); + assertThat("Expected the first array value to be value1", array[0], is("value1")); + assertThat("Expected the second array value to be value2", array[1], is("value2")); + } + + /** + * Assert {@link Settings#getArray(String)} returns {@code null} if the property is not set. + */ + @Test + public void testGetArrayWhereThePropertyIsNotSet() { + // WHEN getting the array + final String[] array = Settings.getArray("key"); + + // THEN null is returned + assertThat("Expected the array to be null", array, nullValue()); + } + /** * Assert {@link Settings#setArrayIfNotEmpty(String, String[])} with an empty array is ignored. */ From 584fd2a47bb5059afd1a1069967ef99a00709fae Mon Sep 17 00:00:00 2001 From: Phillip Whittlesea Date: Sun, 11 Jun 2017 23:26:57 +0100 Subject: [PATCH 5/9] Issue #730: Allow multiple suppression files in Maven The core has been extended to handle multiple suppression files Extended the Ant test to cover multiple suppression files NOTE: This change is breaking for users of the Maven plugin --- .../src/test/resources/build.xml | 3 + .../src/test/resources/test-suppression1.xml | 22 +++- .../src/test/resources/test-suppression2.xml | 41 +++++++ .../analyzer/AbstractSuppressionAnalyzer.java | 101 +++++++++--------- .../src/it/629-jackson-dataformat/pom.xml | 18 ++-- .../invoker.properties | 18 ++++ .../it/730-multiple-suppression-files/pom.xml | 54 ++++++++++ .../postbuild.groovy | 35 ++++++ .../test-suppression1.xml | 27 +++++ .../test-suppression2.xml | 27 +++++ .../maven/BaseDependencyCheckMojo.java | 9 +- 11 files changed, 289 insertions(+), 66 deletions(-) create mode 100644 dependency-check-ant/src/test/resources/test-suppression2.xml create mode 100644 dependency-check-maven/src/it/730-multiple-suppression-files/invoker.properties create mode 100644 dependency-check-maven/src/it/730-multiple-suppression-files/pom.xml create mode 100644 dependency-check-maven/src/it/730-multiple-suppression-files/postbuild.groovy create mode 100644 dependency-check-maven/src/it/730-multiple-suppression-files/test-suppression1.xml create mode 100644 dependency-check-maven/src/it/730-multiple-suppression-files/test-suppression2.xml diff --git a/dependency-check-ant/src/test/resources/build.xml b/dependency-check-ant/src/test/resources/build.xml index d7a2bf845..b48f96154 100644 --- a/dependency-check-ant/src/test/resources/build.xml +++ b/dependency-check-ant/src/test/resources/build.xml @@ -92,6 +92,9 @@ + diff --git a/dependency-check-ant/src/test/resources/test-suppression1.xml b/dependency-check-ant/src/test/resources/test-suppression1.xml index abbc2017d..ff324590e 100644 --- a/dependency-check-ant/src/test/resources/test-suppression1.xml +++ b/dependency-check-ant/src/test/resources/test-suppression1.xml @@ -1,10 +1,26 @@ - + + file name: axis-1.4.jar + ]]> ^org\.apache\.axis:axis:.*$ cpe:/a:apache:axis diff --git a/dependency-check-ant/src/test/resources/test-suppression2.xml b/dependency-check-ant/src/test/resources/test-suppression2.xml new file mode 100644 index 000000000..4fd833855 --- /dev/null +++ b/dependency-check-ant/src/test/resources/test-suppression2.xml @@ -0,0 +1,41 @@ + + + + + + ^jetty:org\.mortbay\.jetty:.*$ + cpe:/a:jetty:jetty + + + + ^jetty:org\.mortbay\.jetty:.*$ + cpe:/a:mortbay:jetty + + + + ^jetty:org\.mortbay\.jetty:.*$ + cpe:/a:mortbay_jetty:jetty + + diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java index 2a24a7892..f2f35a837 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java @@ -117,66 +117,67 @@ public abstract class AbstractSuppressionAnalyzer extends AbstractAnalyzer { return; } - // TODO support more than one file - final String suppressionFilePath = suppressionFilePaths[0]; + for (final String suppressionFilePath : suppressionFilePaths) { + LOGGER.debug("Loading suppression rules from '{}'", suppressionFilePath); - boolean deleteTempFile = false; - try { - final Pattern uriRx = Pattern.compile("^(https?|file)\\:.*", Pattern.CASE_INSENSITIVE); - if (uriRx.matcher(suppressionFilePath).matches()) { - deleteTempFile = true; - file = FileUtils.getTempFile("suppression", "xml"); - final URL url = new URL(suppressionFilePath); - try { - Downloader.fetchFile(url, file, false); - } catch (DownloadFailedException ex) { - Downloader.fetchFile(url, file, true); - } - } else { - file = new File(suppressionFilePath); + boolean deleteTempFile = false; + try { + final Pattern uriRx = Pattern.compile("^(https?|file)\\:.*", Pattern.CASE_INSENSITIVE); + if (uriRx.matcher(suppressionFilePath).matches()) { + deleteTempFile = true; + file = FileUtils.getTempFile("suppression", "xml"); + final URL url = new URL(suppressionFilePath); + try { + Downloader.fetchFile(url, file, false); + } catch (DownloadFailedException ex) { + Downloader.fetchFile(url, file, true); + } + } else { + file = new File(suppressionFilePath); - if (!file.exists()) { - try (InputStream suppressionsFromClasspath = this.getClass().getClassLoader().getResourceAsStream(suppressionFilePath)) { - if (suppressionsFromClasspath != null) { - deleteTempFile = true; - file = FileUtils.getTempFile("suppression", "xml"); - try { - org.apache.commons.io.FileUtils.copyInputStreamToFile(suppressionsFromClasspath, file); - } catch (IOException ex) { - throwSuppressionParseException("Unable to locate suppressions file in classpath", ex); + if (!file.exists()) { + try (InputStream suppressionsFromClasspath = this.getClass().getClassLoader().getResourceAsStream(suppressionFilePath)) { + if (suppressionsFromClasspath != null) { + deleteTempFile = true; + file = FileUtils.getTempFile("suppression", "xml"); + try { + org.apache.commons.io.FileUtils.copyInputStreamToFile(suppressionsFromClasspath, file); + } catch (IOException ex) { + throwSuppressionParseException("Unable to locate suppressions file in classpath", ex); + } } } } } - } - if (file != null) { - if (!file.exists()) { - final String msg = String.format("Suppression file '%s' does not exists", file.getPath()); - LOGGER.warn(msg); - throw new SuppressionParseException(msg); + if (file != null) { + if (!file.exists()) { + final String msg = String.format("Suppression file '%s' does not exists", file.getPath()); + LOGGER.warn(msg); + throw new SuppressionParseException(msg); + } + try { + rules.addAll(parser.parseSuppressionRules(file)); + } catch (SuppressionParseException ex) { + LOGGER.warn("Unable to parse suppression xml file '{}'", file.getPath()); + LOGGER.warn(ex.getMessage()); + throw ex; + } } - try { - rules.addAll(parser.parseSuppressionRules(file)); - LOGGER.debug("{} suppression rules were loaded.", rules.size()); - } catch (SuppressionParseException ex) { - LOGGER.warn("Unable to parse suppression xml file '{}'", file.getPath()); - LOGGER.warn(ex.getMessage()); - throw ex; + } catch (DownloadFailedException ex) { + throwSuppressionParseException("Unable to fetch the configured suppression file", ex); + } catch (MalformedURLException ex) { + throwSuppressionParseException("Configured suppression file has an invalid URL", ex); + } catch (SuppressionParseException ex) { + throw ex; + } catch (IOException ex) { + throwSuppressionParseException("Unable to create temp file for suppressions", ex); + } finally { + if (deleteTempFile && file != null) { + FileUtils.delete(file); } } - } catch (DownloadFailedException ex) { - throwSuppressionParseException("Unable to fetch the configured suppression file", ex); - } catch (MalformedURLException ex) { - throwSuppressionParseException("Configured suppression file has an invalid URL", ex); - } catch (SuppressionParseException ex) { - throw ex; - } catch (IOException ex) { - throwSuppressionParseException("Unable to create temp file for suppressions", ex); - } finally { - if (deleteTempFile && file != null) { - FileUtils.delete(file); - } } + LOGGER.debug("{} suppression rules were loaded.", rules.size()); } /** diff --git a/dependency-check-maven/src/it/629-jackson-dataformat/pom.xml b/dependency-check-maven/src/it/629-jackson-dataformat/pom.xml index f1e6cd154..7c9b78e60 100644 --- a/dependency-check-maven/src/it/629-jackson-dataformat/pom.xml +++ b/dependency-check-maven/src/it/629-jackson-dataformat/pom.xml @@ -22,8 +22,8 @@ Copyright (c) 2013 Jeremy Long. All Rights Reserved. test-dataformat-jackson 1.0.0-SNAPSHOT jar - - + + com.fasterxml.jackson.core jackson-databind 2.4.5 @@ -38,10 +38,10 @@ Copyright (c) 2013 Jeremy Long. All Rights Reserved. jackson-dataformat-cbor 2.4.5 - - com.fasterxml.jackson.dataformat - jackson-dataformat-xml - 2.4.5 - - - \ No newline at end of file + + com.fasterxml.jackson.dataformat + jackson-dataformat-xml + 2.4.5 + + + diff --git a/dependency-check-maven/src/it/730-multiple-suppression-files/invoker.properties b/dependency-check-maven/src/it/730-multiple-suppression-files/invoker.properties new file mode 100644 index 000000000..9c2542d23 --- /dev/null +++ b/dependency-check-maven/src/it/730-multiple-suppression-files/invoker.properties @@ -0,0 +1,18 @@ +# +# This file is part of dependency-check-core. +# +# 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. +# +# Copyright (c) 2017 The OWASP Foundation. All Rights Reserved. +# +invoker.goals = install ${project.groupId}:${project.artifactId}:${project.version}:check diff --git a/dependency-check-maven/src/it/730-multiple-suppression-files/pom.xml b/dependency-check-maven/src/it/730-multiple-suppression-files/pom.xml new file mode 100644 index 000000000..8a534171e --- /dev/null +++ b/dependency-check-maven/src/it/730-multiple-suppression-files/pom.xml @@ -0,0 +1,54 @@ + + + + 4.0.0 + org.owasp.test + test-multiple-suppression-files + 1.0.0-SNAPSHOT + jar + + + + + com.vaadin.external.google + android-json + 0.0.20131108.vaadin1 + + + com.fasterxml.jackson.dataformat + jackson-dataformat-xml + 2.4.5 + + + + + + + org.owasp + dependency-check-maven + + + ${project.basedir}/test-suppression1.xml + ${project.basedir}/test-suppression2.xml + + + + + + diff --git a/dependency-check-maven/src/it/730-multiple-suppression-files/postbuild.groovy b/dependency-check-maven/src/it/730-multiple-suppression-files/postbuild.groovy new file mode 100644 index 000000000..2a42192d9 --- /dev/null +++ b/dependency-check-maven/src/it/730-multiple-suppression-files/postbuild.groovy @@ -0,0 +1,35 @@ +/* + * This file is part of dependency-check-core. + * + * 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. + * + * Copyright (c) 2017 The OWASP Foundation. All Rights Reserved. + */ + +import org.apache.commons.io.FileUtils +import org.apache.commons.lang.StringUtils + +import java.nio.charset.Charset + +// Check that suppression worked. +String log = FileUtils.readFileToString(new File(basedir, "build.log"), Charset.defaultCharset().name()); +int count = StringUtils.countMatches(log, "CVE-2016-5696"); +if (count > 0) { + System.out.println(String.format("CVE-2016-5696 (android-json-0.0.20131108.vaadin1.jar) was identified and should be suppressed")); + return false; +} +count = StringUtils.countMatches(log, "CVE-2016-7051"); +if (count > 0) { + System.out.println(String.format("CVE-2016-7051 (jackson-module-jaxb-annotations-2.4.5.jar) was identified and should be suppressed")); + return false; +} diff --git a/dependency-check-maven/src/it/730-multiple-suppression-files/test-suppression1.xml b/dependency-check-maven/src/it/730-multiple-suppression-files/test-suppression1.xml new file mode 100644 index 000000000..e3bd8c0b0 --- /dev/null +++ b/dependency-check-maven/src/it/730-multiple-suppression-files/test-suppression1.xml @@ -0,0 +1,27 @@ + + + + + + ^com\.vaadin\.external\.google:android-json:.*$ + cpe:/a:google:android + + diff --git a/dependency-check-maven/src/it/730-multiple-suppression-files/test-suppression2.xml b/dependency-check-maven/src/it/730-multiple-suppression-files/test-suppression2.xml new file mode 100644 index 000000000..f0b4fad7b --- /dev/null +++ b/dependency-check-maven/src/it/730-multiple-suppression-files/test-suppression2.xml @@ -0,0 +1,27 @@ + + + + + + ^com\.fasterxml\.jackson.*:.*:.*$ + cpe:/a:fasterxml:jackson + + diff --git a/dependency-check-maven/src/main/java/org/owasp/dependencycheck/maven/BaseDependencyCheckMojo.java b/dependency-check-maven/src/main/java/org/owasp/dependencycheck/maven/BaseDependencyCheckMojo.java index eb945c2ab..0504bdfd5 100644 --- a/dependency-check-maven/src/main/java/org/owasp/dependencycheck/maven/BaseDependencyCheckMojo.java +++ b/dependency-check-maven/src/main/java/org/owasp/dependencycheck/maven/BaseDependencyCheckMojo.java @@ -199,10 +199,10 @@ public abstract class BaseDependencyCheckMojo extends AbstractMojo implements Ma @Parameter(property = "connectionTimeout", defaultValue = "", required = false) private String connectionTimeout; /** - * The path to the suppression file. + * The paths to the suppression files. */ - @Parameter(property = "suppressionFile", defaultValue = "", required = false) - private String suppressionFile; + @Parameter(required = false) + private String[] suppressionFiles; /** * The path to the hints file. @@ -920,8 +920,9 @@ public abstract class BaseDependencyCheckMojo extends AbstractMojo implements Ma Settings.setStringIfNotNull(Settings.KEYS.PROXY_NON_PROXY_HOSTS, proxy.getNonProxyHosts()); } + Settings.setArrayIfNotEmpty(Settings.KEYS.SUPPRESSION_FILE, suppressionFiles); + Settings.setStringIfNotEmpty(Settings.KEYS.CONNECTION_TIMEOUT, connectionTimeout); - Settings.setStringIfNotEmpty(Settings.KEYS.SUPPRESSION_FILE, suppressionFile); Settings.setStringIfNotEmpty(Settings.KEYS.HINTS_FILE, hintsFile); //File Type Analyzer Settings From 8021aaed4b8262f39cee088af224b2a654ae9473 Mon Sep 17 00:00:00 2001 From: Phillip Whittlesea Date: Mon, 12 Jun 2017 01:18:10 +0100 Subject: [PATCH 6/9] Issue #730: Core tests for multiple suppression files Added updates to Maven plugin documentation Added upgrade notes to the README --- README.md | 63 +++++++++++ .../owasp/dependencycheck/taskdefs/Check.java | 2 +- .../taskdefs/DependencyCheckTaskTest.java | 2 +- .../AbstractSuppressionAnalyzerTest.java | 100 ++++++++++++------ .../src/test/resources/other-suppressions.xml | 27 +++++ .../it/730-multiple-suppression-files/pom.xml | 4 +- .../src/site/markdown/configuration.md | 2 +- .../src/site/markdown/index.md.vm | 36 +++++++ 8 files changed, 201 insertions(+), 35 deletions(-) create mode 100644 dependency-check-core/src/test/resources/other-suppressions.xml diff --git a/README.md b/README.md index c924b00c2..d9db978e7 100644 --- a/README.md +++ b/README.md @@ -136,6 +136,69 @@ docker run --rm \ ``` +Upgrade Notes +------------- + +### Upgrading from **1.x.x** to **2.x.x** + +Note that when upgrading from version 1.x.x that the following changes will need to be made to your configuration. + +#### Suppression file + +In order to support multiple suppression files, the mechanism for configuring suppression files has changed. +As such, users that have defined a suppression file in their configuration will need to update. + +See the examples below: + +##### Ant + +Old: + +```xml + + +``` + +New: + +```xml + + suppression.xml + +``` + +##### Maven + +Old: + +```xml + + org.owasp + dependency-check-maven + + suppression.xml + + +``` + +New: + +```xml + + org.owasp + dependency-check-maven + + + suppression.xml + + + +``` + + Mailing List ------------ diff --git a/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java b/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java index 65b6cd9a8..5af32b863 100644 --- a/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java +++ b/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java @@ -461,7 +461,7 @@ public class Check extends Update { */ @Deprecated public void setSuppressionFile(String suppressionFile) { - throw new BuildException("Property form of suppressionFile has been replaced by a nested element, please update your configuration."); + throw new BuildException("Definition of a suppression file via a property has been deprecated. Suppression files are now defined as a nested element, please update your configuration."); } /** diff --git a/dependency-check-ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskTest.java b/dependency-check-ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskTest.java index 87beaf6d9..dfb4c86e5 100644 --- a/dependency-check-ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskTest.java +++ b/dependency-check-ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskTest.java @@ -140,7 +140,7 @@ public class DependencyCheckTaskTest { // WHEN executing the ant task // THEN an exception with a warning is thrown expectedException.expect(BuildException.class); - expectedException.expectMessage("Property form of suppressionFile has been replaced by a nested element, please update your configuration."); + expectedException.expectMessage("Definition of a suppression file via a property has been deprecated. Suppression files are now defined as a nested element, please update your configuration."); buildFileRule.executeTarget(antTaskName); } } diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzerTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzerTest.java index 119d90bd0..704cf32dd 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzerTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzerTest.java @@ -17,30 +17,34 @@ */ package org.owasp.dependencycheck.analyzer; +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; + +import java.util.Set; + import org.junit.Before; import org.junit.Test; import org.owasp.dependencycheck.BaseTest; import org.owasp.dependencycheck.Engine; import org.owasp.dependencycheck.analyzer.exception.AnalysisException; import org.owasp.dependencycheck.dependency.Dependency; -import org.owasp.dependencycheck.xml.suppression.SuppressionRule; -import org.owasp.dependencycheck.utils.Settings; -import org.slf4j.LoggerFactory; - -import java.net.MalformedURLException; -import java.net.URISyntaxException; -import java.util.List; -import java.util.Set; - -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; import org.owasp.dependencycheck.exception.InitializationException; +import org.owasp.dependencycheck.utils.Settings; +import org.owasp.dependencycheck.utils.Settings.KEYS; /** * @author Jeremy Long */ public class AbstractSuppressionAnalyzerTest extends BaseTest { + /** A second suppression file to test with. */ + private static final String OTHER_SUPPRESSIONS_FILE = "other-suppressions.xml"; + + /** Suppression file to test with. */ + private static final String SUPPRESSIONS_FILE = "suppressions.xml"; + private AbstractSuppressionAnalyzer instance; @Before @@ -64,24 +68,42 @@ public class AbstractSuppressionAnalyzerTest extends BaseTest { */ @Test public void testGetRulesFromSuppressionFileFromURL() throws Exception { - setSupressionFileFromURL(); - instance.initialize(); - int expCount = 5; - List result = instance.getRules(); - assertTrue(expCount <= result.size()); + final String fileUrl = getClass().getClassLoader().getResource(SUPPRESSIONS_FILE).toURI().toURL().toString(); + final int numberOfExtraLoadedRules = getNumberOfRulesLoadedFromPath(fileUrl) - getNumberOfRulesLoadedInCoreFile(); + assertEquals("Expected 5 extra rules in the given path", 5, numberOfExtraLoadedRules); } /** * Test of getRules method, of class AbstractSuppressionAnalyzer for - * suppression file declared as URL. + * suppression file on the classpath. */ @Test public void testGetRulesFromSuppressionFileInClasspath() throws Exception { - Settings.setString(Settings.KEYS.SUPPRESSION_FILE, "suppressions.xml"); + final int numberOfExtraLoadedRules = getNumberOfRulesLoadedFromPath(SUPPRESSIONS_FILE) - getNumberOfRulesLoadedInCoreFile(); + assertEquals("Expected 5 extra rules in the given file", 5, numberOfExtraLoadedRules); + } + + /** + * Assert that rules are loaded from multiple files if multiple files are denfined in the {@link Settings} singleton. + */ + @Test + public void testGetRulesFromMultipleSuppressionFiles() throws Exception { + final int rulesInCoreFile = getNumberOfRulesLoadedInCoreFile(); + + // GIVEN suppression rules from one file + final int rulesInFirstFile = getNumberOfRulesLoadedFromPath(SUPPRESSIONS_FILE) - rulesInCoreFile; + + // AND suppression rules from another file + final int rulesInSecondFile = getNumberOfRulesLoadedFromPath(OTHER_SUPPRESSIONS_FILE) - rulesInCoreFile; + + // WHEN initializing with both suppression files + final String[] suppressionFiles = { SUPPRESSIONS_FILE, OTHER_SUPPRESSIONS_FILE }; + Settings.setArrayIfNotEmpty(KEYS.SUPPRESSION_FILE, suppressionFiles); instance.initialize(); - int expCount = 5; - int currentSize = instance.getRules().size(); - assertTrue(expCount <= currentSize); + + // THEN rules from both files were loaded + final int expectedSize = rulesInFirstFile + rulesInSecondFile + rulesInCoreFile; + assertThat("Expected suppressions from both files", instance.getRules().size(), is(expectedSize)); } @Test(expected = InitializationException.class) @@ -90,15 +112,33 @@ public class AbstractSuppressionAnalyzerTest extends BaseTest { instance.initialize(); } - private void setSupressionFileFromURL() throws Exception { - try { - final String uri = this.getClass().getClassLoader().getResource("suppressions.xml").toURI().toURL().toString(); - Settings.setString(Settings.KEYS.SUPPRESSION_FILE, uri); - } catch (URISyntaxException ex) { - LoggerFactory.getLogger(AbstractSuppressionAnalyzerTest.class).error("", ex); - } catch (MalformedURLException ex) { - LoggerFactory.getLogger(AbstractSuppressionAnalyzerTest.class).error("", ex); - } + /** + * Return the number of rules that are loaded from the core suppression file. + * + * @return the number of rules defined in the core suppresion file. + * @throws Exception if loading the rules fails. + */ + private int getNumberOfRulesLoadedInCoreFile() throws Exception { + Settings.removeProperty(KEYS.SUPPRESSION_FILE); + + final AbstractSuppressionAnalyzerImpl coreFileAnalyzer = new AbstractSuppressionAnalyzerImpl(); + coreFileAnalyzer.initialize(); + return coreFileAnalyzer.getRules().size(); + } + + /** + * Load a file into the {@link AbstractSuppressionAnalyzer} and return the number of rules loaded. + * + * @param path the path to load. + * @return the number of rules that were loaded (including the core rules). + * @throws Exception if loading the rules fails. + */ + private int getNumberOfRulesLoadedFromPath(final String path) throws Exception { + Settings.setString(KEYS.SUPPRESSION_FILE, path); + + final AbstractSuppressionAnalyzerImpl fileAnalyzer = new AbstractSuppressionAnalyzerImpl(); + fileAnalyzer.initialize(); + return fileAnalyzer.getRules().size(); } public class AbstractSuppressionAnalyzerImpl extends AbstractSuppressionAnalyzer { diff --git a/dependency-check-core/src/test/resources/other-suppressions.xml b/dependency-check-core/src/test/resources/other-suppressions.xml new file mode 100644 index 000000000..f0b4fad7b --- /dev/null +++ b/dependency-check-core/src/test/resources/other-suppressions.xml @@ -0,0 +1,27 @@ + + + + + + ^com\.fasterxml\.jackson.*:.*:.*$ + cpe:/a:fasterxml:jackson + + diff --git a/dependency-check-maven/src/it/730-multiple-suppression-files/pom.xml b/dependency-check-maven/src/it/730-multiple-suppression-files/pom.xml index 8a534171e..1b64e9518 100644 --- a/dependency-check-maven/src/it/730-multiple-suppression-files/pom.xml +++ b/dependency-check-maven/src/it/730-multiple-suppression-files/pom.xml @@ -44,8 +44,8 @@ Copyright (c) 2017 The OWASP Foundation. All Rights Reserved. dependency-check-maven - ${project.basedir}/test-suppression1.xml - ${project.basedir}/test-suppression2.xml + ${project.basedir}/test-suppression1.xml + ${project.basedir}/test-suppression2.xml diff --git a/dependency-check-maven/src/site/markdown/configuration.md b/dependency-check-maven/src/site/markdown/configuration.md index 08dfab16c..e0e7b38a8 100644 --- a/dependency-check-maven/src/site/markdown/configuration.md +++ b/dependency-check-maven/src/site/markdown/configuration.md @@ -28,7 +28,7 @@ skipRuntimeScope | Skip analysis for artifacts with Runtime Scope. skipSystemScope | Skip analysis for artifacts with System Scope. | false skipTestScope | Skip analysis for artifacts with Test Scope. | true skipArtifactType | A regular expression used to filter/skip artifact types. |   -suppressionFile | The file path to the XML suppression file \- used to suppress [false positives](../general/suppression.html). |   +suppressionFiles | The file paths to the XML suppression files \- used to suppress [false positives](../general/suppression.html). |   hintsFile | The file path to the XML hints file \- used to resolve [false negatives](../general/hints.html). |   enableExperimental | Enable the [experimental analyzers](../analyzers/index.html). If not enabled the experimental analyzers (see below) will not be loaded or used. | false diff --git a/dependency-check-maven/src/site/markdown/index.md.vm b/dependency-check-maven/src/site/markdown/index.md.vm index 6490307fe..43815f4ab 100644 --- a/dependency-check-maven/src/site/markdown/index.md.vm +++ b/dependency-check-maven/src/site/markdown/index.md.vm @@ -204,3 +204,39 @@ Update the local cache of the NVD data from NIST without analyzing the dependenc ... ``` + +$H$H$H Example 7: +Suppress false positives using multiple suppression files (E.g. a company-wide suppression file and a local project file). + +```xml + + ... + + ... + + ... + + org.owasp + dependency-check-maven + ${project.version} + + + http://example.org/suppression.xml + project-suppression.xml + + + + + + check + + + + + ... + + ... + + ... + +``` From 0be494a2111a75ea80e18bf8fe27000cdcb9163d Mon Sep 17 00:00:00 2001 From: Phillip Whittlesea Date: Mon, 12 Jun 2017 01:48:33 +0100 Subject: [PATCH 7/9] Issue #730: Remove redundant method from Settings Pull file loading from loadSuppressionData() to make it easier to read Add test assertion to happy case Ant task test --- .../taskdefs/DependencyCheckTaskTest.java | 2 + .../analyzer/AbstractSuppressionAnalyzer.java | 116 ++++++++++-------- .../owasp/dependencycheck/utils/Settings.java | 14 +-- .../dependencycheck/utils/SettingsTest.java | 15 --- 4 files changed, 67 insertions(+), 80 deletions(-) diff --git a/dependency-check-ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskTest.java b/dependency-check-ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskTest.java index dfb4c86e5..e1c41c9e8 100644 --- a/dependency-check-ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskTest.java +++ b/dependency-check-ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskTest.java @@ -127,6 +127,8 @@ public class DependencyCheckTaskTest { buildFileRule.executeTarget(antTaskName); // THEN the ant task executed without error + final File report = new File("target/dependency-check-report.html"); + assertTrue("Expected the DependencyCheck report to be generated", report.exists()); } /** diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java index f2f35a837..b8f41da5c 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java @@ -99,13 +99,12 @@ public abstract class AbstractSuppressionAnalyzer extends AbstractAnalyzer { } /** - * Loads the suppression rules file. + * Loads all the suppression rules files configured in the {@link Settings} singleton. * * @throws SuppressionParseException thrown if the XML cannot be parsed. */ private void loadSuppressionData() throws SuppressionParseException { final SuppressionParser parser = new SuppressionParser(); - File file = null; try { final InputStream in = this.getClass().getClassLoader().getResourceAsStream("dependencycheck-base-suppression.xml"); rules = parser.parseSuppressionRules(in); @@ -117,67 +116,80 @@ public abstract class AbstractSuppressionAnalyzer extends AbstractAnalyzer { return; } + // Load all the suppression file paths for (final String suppressionFilePath : suppressionFilePaths) { - LOGGER.debug("Loading suppression rules from '{}'", suppressionFilePath); + loadSuppressionFile(parser, suppressionFilePath); + } + LOGGER.debug("{} suppression rules were loaded.", rules.size()); + } - boolean deleteTempFile = false; - try { - final Pattern uriRx = Pattern.compile("^(https?|file)\\:.*", Pattern.CASE_INSENSITIVE); - if (uriRx.matcher(suppressionFilePath).matches()) { - deleteTempFile = true; - file = FileUtils.getTempFile("suppression", "xml"); - final URL url = new URL(suppressionFilePath); - try { - Downloader.fetchFile(url, file, false); - } catch (DownloadFailedException ex) { - Downloader.fetchFile(url, file, true); - } - } else { - file = new File(suppressionFilePath); + /** + * Load a single suppression rules file from the path provided using the parser provided. + * + * @param parser the parser to use for loading the file. + * @param suppressionFilePath the path to load. + * @throws SuppressionParseException thrown if the suppression file cannot be loaded and parsed. + */ + private void loadSuppressionFile(final SuppressionParser parser, final String suppressionFilePath) throws SuppressionParseException { + LOGGER.debug("Loading suppression rules from '{}'", suppressionFilePath); - if (!file.exists()) { - try (InputStream suppressionsFromClasspath = this.getClass().getClassLoader().getResourceAsStream(suppressionFilePath)) { - if (suppressionsFromClasspath != null) { - deleteTempFile = true; - file = FileUtils.getTempFile("suppression", "xml"); - try { - org.apache.commons.io.FileUtils.copyInputStreamToFile(suppressionsFromClasspath, file); - } catch (IOException ex) { - throwSuppressionParseException("Unable to locate suppressions file in classpath", ex); - } + File file = null; + boolean deleteTempFile = false; + try { + final Pattern uriRx = Pattern.compile("^(https?|file)\\:.*", Pattern.CASE_INSENSITIVE); + if (uriRx.matcher(suppressionFilePath).matches()) { + deleteTempFile = true; + file = FileUtils.getTempFile("suppression", "xml"); + final URL url = new URL(suppressionFilePath); + try { + Downloader.fetchFile(url, file, false); + } catch (DownloadFailedException ex) { + Downloader.fetchFile(url, file, true); + } + } else { + file = new File(suppressionFilePath); + + if (!file.exists()) { + try (InputStream suppressionsFromClasspath = this.getClass().getClassLoader().getResourceAsStream(suppressionFilePath)) { + if (suppressionsFromClasspath != null) { + deleteTempFile = true; + file = FileUtils.getTempFile("suppression", "xml"); + try { + org.apache.commons.io.FileUtils.copyInputStreamToFile(suppressionsFromClasspath, file); + } catch (IOException ex) { + throwSuppressionParseException("Unable to locate suppressions file in classpath", ex); } } } } - if (file != null) { - if (!file.exists()) { - final String msg = String.format("Suppression file '%s' does not exists", file.getPath()); - LOGGER.warn(msg); - throw new SuppressionParseException(msg); - } - try { - rules.addAll(parser.parseSuppressionRules(file)); - } catch (SuppressionParseException ex) { - LOGGER.warn("Unable to parse suppression xml file '{}'", file.getPath()); - LOGGER.warn(ex.getMessage()); - throw ex; - } + } + if (file != null) { + if (!file.exists()) { + final String msg = String.format("Suppression file '%s' does not exists", file.getPath()); + LOGGER.warn(msg); + throw new SuppressionParseException(msg); } - } catch (DownloadFailedException ex) { - throwSuppressionParseException("Unable to fetch the configured suppression file", ex); - } catch (MalformedURLException ex) { - throwSuppressionParseException("Configured suppression file has an invalid URL", ex); - } catch (SuppressionParseException ex) { - throw ex; - } catch (IOException ex) { - throwSuppressionParseException("Unable to create temp file for suppressions", ex); - } finally { - if (deleteTempFile && file != null) { - FileUtils.delete(file); + try { + rules.addAll(parser.parseSuppressionRules(file)); + } catch (SuppressionParseException ex) { + LOGGER.warn("Unable to parse suppression xml file '{}'", file.getPath()); + LOGGER.warn(ex.getMessage()); + throw ex; } } + } catch (DownloadFailedException ex) { + throwSuppressionParseException("Unable to fetch the configured suppression file", ex); + } catch (MalformedURLException ex) { + throwSuppressionParseException("Configured suppression file has an invalid URL", ex); + } catch (SuppressionParseException ex) { + throw ex; + } catch (IOException ex) { + throwSuppressionParseException("Unable to create temp file for suppressions", ex); + } finally { + if (deleteTempFile && file != null) { + FileUtils.delete(file); + } } - LOGGER.debug("{} suppression rules were loaded.", rules.size()); } /** diff --git a/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java b/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java index 32aa3147b..a24971df0 100644 --- a/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java +++ b/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java @@ -564,18 +564,6 @@ public final class Settings { LOGGER.debug("Setting: {}='{}'", key, value); } - /** - * Sets a property value from an array. - *

- * Note: each value of the array will be joined by the delimiter {@link Settings#ARRAY_SEP}. - * - * @param key the key for the property - * @param value the value for the property - */ - static void setArray(String key, String[] value) { - setString(key, StringUtils.join(value, ARRAY_SEP)); - } - /** * Sets a property value only if the value is not null. * @@ -608,7 +596,7 @@ public final class Settings { */ public static void setArrayIfNotEmpty(String key, String[] value) { if (null != value && value.length > 0) { - setArray(key, value); + setString(key, StringUtils.join(value, ARRAY_SEP)); } } diff --git a/dependency-check-utils/src/test/java/org/owasp/dependencycheck/utils/SettingsTest.java b/dependency-check-utils/src/test/java/org/owasp/dependencycheck/utils/SettingsTest.java index 881be4138..b91333f3e 100644 --- a/dependency-check-utils/src/test/java/org/owasp/dependencycheck/utils/SettingsTest.java +++ b/dependency-check-utils/src/test/java/org/owasp/dependencycheck/utils/SettingsTest.java @@ -336,19 +336,4 @@ public class SettingsTest extends BaseTest { assertThat("Expected the property to be set", Settings.getString("key"), is("value1")); } - /** - * Assert {@link Settings#setArray(String, String[])} with multiple values sets a delimited string. - */ - @Test - public void testSetArraySetsADelimitedString() { - // GIVEN an array with values - final String[] array = { "value1", "value2" }; - - // WHEN setting the array - Settings.setArray("key", array); - - // THEN the property is set - assertThat("Expected the property to be set", Settings.getString("key"), is("value1,value2")); - } - } From 3d5b86d96f40ad7d5a13f729cdc88aedaab723ff Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Tue, 20 Jun 2017 06:57:58 -0400 Subject: [PATCH 8/9] minor formating updates --- .../src/test/resources/build.xml | 32 +++++++++---------- .../org/owasp/dependencycheck/AppTest.java | 32 +++++++++++-------- .../src/it/629-jackson-dataformat/pom.xml | 2 +- .../dependencycheck/utils/SettingsTest.java | 23 +++++++------ 4 files changed, 49 insertions(+), 40 deletions(-) diff --git a/dependency-check-ant/src/test/resources/build.xml b/dependency-check-ant/src/test/resources/build.xml index b48f96154..21f7a84a0 100644 --- a/dependency-check-ant/src/test/resources/build.xml +++ b/dependency-check-ant/src/test/resources/build.xml @@ -61,11 +61,11 @@ + applicationName="test failCVSS" + reportOutputDirectory="${project.build.directory}" + reportFormat="XML" + autoupdate="false" + failBuildOnCVSS="3"> @@ -74,27 +74,27 @@ + applicationName="test suppression" + reportOutputDirectory="${project.build.directory}" + autoupdate="false" + failBuildOnCVSS="3" + suppressionFile="${project.build.directory}/test-classes/test-suppression1.xml"/> + applicationName="test suppression" + reportOutputDirectory="${project.build.directory}" + autoupdate="false" + failBuildOnCVSS="3"> ${project.build.directory}/test-classes/test-suppression1.xml ${project.build.directory}/test-classes/test-suppression2.xml + dir="${project.build.directory}/test-classes/list" + files="jetty-6.1.0.jar,org.mortbay.jetty.jar"/> diff --git a/dependency-check-cli/src/test/java/org/owasp/dependencycheck/AppTest.java b/dependency-check-cli/src/test/java/org/owasp/dependencycheck/AppTest.java index 0365bf296..4a2346a31 100644 --- a/dependency-check-cli/src/test/java/org/owasp/dependencycheck/AppTest.java +++ b/dependency-check-cli/src/test/java/org/owasp/dependencycheck/AppTest.java @@ -44,7 +44,9 @@ import org.owasp.dependencycheck.utils.Settings.KEYS; */ public class AppTest { - /** Test rule for asserting exceptions and their contents. */ + /** + * Test rule for asserting exceptions and their contents. + */ @Rule public ExpectedException expectedException = ExpectedException.none(); @@ -82,65 +84,67 @@ public class AppTest { } /** - * Assert that boolean properties can be set on the CLI and parsed into the {@link Settings} singleton. + * Assert that boolean properties can be set on the CLI and parsed into the + * {@link Settings} singleton. * * @throws Exception the unexpected {@link Exception}. */ @Test public void testPopulateSettings() throws Exception { File prop = new File(this.getClass().getClassLoader().getResource("sample.properties").toURI().getPath()); - String[] args = { "-P", prop.getAbsolutePath() }; + String[] args = {"-P", prop.getAbsolutePath()}; Map expected = new HashMap<>(); expected.put(Settings.KEYS.AUTO_UPDATE, Boolean.FALSE); expected.put(Settings.KEYS.ANALYZER_ARCHIVE_ENABLED, Boolean.TRUE); assertTrue(testBooleanProperties(args, expected)); - String[] args2 = { "-n" }; + String[] args2 = {"-n"}; expected.put(Settings.KEYS.AUTO_UPDATE, Boolean.FALSE); expected.put(Settings.KEYS.ANALYZER_ARCHIVE_ENABLED, Boolean.TRUE); assertTrue(testBooleanProperties(args2, expected)); - String[] args3 = { "-h" }; + String[] args3 = {"-h"}; expected.put(Settings.KEYS.AUTO_UPDATE, Boolean.TRUE); expected.put(Settings.KEYS.ANALYZER_ARCHIVE_ENABLED, Boolean.TRUE); assertTrue(testBooleanProperties(args3, expected)); - String[] args4 = { "--disableArchive" }; + String[] args4 = {"--disableArchive"}; expected.put(Settings.KEYS.AUTO_UPDATE, Boolean.TRUE); expected.put(Settings.KEYS.ANALYZER_ARCHIVE_ENABLED, Boolean.FALSE); assertTrue(testBooleanProperties(args4, expected)); - String[] args5 = { "-P", prop.getAbsolutePath(), "--disableArchive" }; + String[] args5 = {"-P", prop.getAbsolutePath(), "--disableArchive"}; expected.put(Settings.KEYS.AUTO_UPDATE, Boolean.FALSE); expected.put(Settings.KEYS.ANALYZER_ARCHIVE_ENABLED, Boolean.FALSE); assertTrue(testBooleanProperties(args5, expected)); prop = new File(this.getClass().getClassLoader().getResource("sample2.properties").toURI().getPath()); - String[] args6 = { "-P", prop.getAbsolutePath(), "--disableArchive" }; + String[] args6 = {"-P", prop.getAbsolutePath(), "--disableArchive"}; expected.put(Settings.KEYS.AUTO_UPDATE, Boolean.TRUE); expected.put(Settings.KEYS.ANALYZER_ARCHIVE_ENABLED, Boolean.FALSE); assertTrue(testBooleanProperties(args6, expected)); - String[] args7 = { "-P", prop.getAbsolutePath(), "--noupdate" }; + String[] args7 = {"-P", prop.getAbsolutePath(), "--noupdate"}; expected.put(Settings.KEYS.AUTO_UPDATE, Boolean.FALSE); expected.put(Settings.KEYS.ANALYZER_ARCHIVE_ENABLED, Boolean.FALSE); assertTrue(testBooleanProperties(args7, expected)); - String[] args8 = { "-P", prop.getAbsolutePath(), "--noupdate", "--disableArchive" }; + String[] args8 = {"-P", prop.getAbsolutePath(), "--noupdate", "--disableArchive"}; expected.put(Settings.KEYS.AUTO_UPDATE, Boolean.FALSE); expected.put(Settings.KEYS.ANALYZER_ARCHIVE_ENABLED, Boolean.FALSE); assertTrue(testBooleanProperties(args8, expected)); } /** - * Assert that an {@link UnrecognizedOptionException} is thrown when a property that is not supported is specified on the CLI. + * Assert that an {@link UnrecognizedOptionException} is thrown when a + * property that is not supported is specified on the CLI. * * @throws Exception the unexpected {@link Exception}. */ @Test public void testPopulateSettingsException() throws Exception { - String[] args = { "-invalidPROPERTY" }; + String[] args = {"-invalidPROPERTY"}; expectedException.expect(UnrecognizedOptionException.class); expectedException.expectMessage("Unrecognized option: -invalidPROPERTY"); @@ -158,7 +162,7 @@ public class AppTest { File prop = new File(this.getClass().getClassLoader().getResource("sample.properties").toURI().getPath()); // AND a single suppression file - String[] args = { "-P", prop.getAbsolutePath(), "--suppression", "another-file.xml" }; + String[] args = {"-P", prop.getAbsolutePath(), "--suppression", "another-file.xml"}; // WHEN parsing the CLI arguments final CliParser cli = new CliParser(); @@ -181,7 +185,7 @@ public class AppTest { File prop = new File(this.getClass().getClassLoader().getResource("sample.properties").toURI().getPath()); // AND a single suppression file - String[] args = { "-P", prop.getAbsolutePath(), "--suppression", "first-file.xml", "another-file.xml" }; + String[] args = {"-P", prop.getAbsolutePath(), "--suppression", "first-file.xml", "another-file.xml"}; // WHEN parsing the CLI arguments final CliParser cli = new CliParser(); diff --git a/dependency-check-maven/src/it/629-jackson-dataformat/pom.xml b/dependency-check-maven/src/it/629-jackson-dataformat/pom.xml index 7c9b78e60..0c7cce1f4 100644 --- a/dependency-check-maven/src/it/629-jackson-dataformat/pom.xml +++ b/dependency-check-maven/src/it/629-jackson-dataformat/pom.xml @@ -14,7 +14,7 @@ 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. -Copyright (c) 2013 Jeremy Long. All Rights Reserved. +Copyright (c) 2017 Jeremy Long. All Rights Reserved. --> 4.0.0 diff --git a/dependency-check-utils/src/test/java/org/owasp/dependencycheck/utils/SettingsTest.java b/dependency-check-utils/src/test/java/org/owasp/dependencycheck/utils/SettingsTest.java index b91333f3e..a9087f9e7 100644 --- a/dependency-check-utils/src/test/java/org/owasp/dependencycheck/utils/SettingsTest.java +++ b/dependency-check-utils/src/test/java/org/owasp/dependencycheck/utils/SettingsTest.java @@ -246,7 +246,8 @@ public class SettingsTest extends BaseTest { } /** - * Assert {@link Settings#getArray(String)} from a delimited string returns multiple values in an array. + * Assert {@link Settings#getArray(String)} from a delimited string returns + * multiple values in an array. */ @Test public void testGetArrayFromADelimitedString() { @@ -265,7 +266,8 @@ public class SettingsTest extends BaseTest { } /** - * Assert {@link Settings#getArray(String)} returns {@code null} if the property is not set. + * Assert {@link Settings#getArray(String)} returns {@code null} if the + * property is not set. */ @Test public void testGetArrayWhereThePropertyIsNotSet() { @@ -277,7 +279,8 @@ public class SettingsTest extends BaseTest { } /** - * Assert {@link Settings#setArrayIfNotEmpty(String, String[])} with an empty array is ignored. + * Assert {@link Settings#setArrayIfNotEmpty(String, String[])} with an + * empty array is ignored. */ @Test public void testSetArrayNotEmptyIgnoresAnEmptyArray() { @@ -292,7 +295,8 @@ public class SettingsTest extends BaseTest { } /** - * Assert {@link Settings#setArrayIfNotEmpty(String, String[])} with a null array is ignored. + * Assert {@link Settings#setArrayIfNotEmpty(String, String[])} with a null + * array is ignored. */ @Test public void testSetArrayNotEmptyIgnoresAnNullArray() { @@ -307,12 +311,13 @@ public class SettingsTest extends BaseTest { } /** - * Assert {@link Settings#setArrayIfNotEmpty(String, String[])} with multiple values sets a delimited string. + * Assert {@link Settings#setArrayIfNotEmpty(String, String[])} with + * multiple values sets a delimited string. */ @Test public void testSetArrayNotEmptySetsADelimitedString() { // GIVEN an array with values - final String[] array = { "value1", "value2" }; + final String[] array = {"value1", "value2"}; // WHEN setting the array Settings.setArrayIfNotEmpty("key", array); @@ -322,12 +327,13 @@ public class SettingsTest extends BaseTest { } /** - * Assert {@link Settings#setArrayIfNotEmpty(String, String[])} with a single values sets a string. + * Assert {@link Settings#setArrayIfNotEmpty(String, String[])} with a + * single values sets a string. */ @Test public void testSetArrayNotEmptyWithSingleValueSetsAString() { // GIVEN an array with a value - final String[] array = { "value1" }; + final String[] array = {"value1"}; // WHEN setting the array Settings.setArrayIfNotEmpty("key", array); @@ -335,5 +341,4 @@ public class SettingsTest extends BaseTest { // THEN the property is set assertThat("Expected the property to be set", Settings.getString("key"), is("value1")); } - } From dee1ccfd3e5fab518a718b71c7f4f7d269ff2c5d Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Thu, 22 Jun 2017 07:18:14 -0400 Subject: [PATCH 9/9] updates to allow old suppression file configuration --- .../owasp/dependencycheck/taskdefs/Check.java | 13 +++-- .../taskdefs/SuppressionFile.java | 6 ++- .../taskdefs/DependencyCheckTaskTest.java | 35 +++++++++--- .../src/test/resources/build.xml | 37 +++++++++---- .../org/owasp/dependencycheck/CliParser.java | 6 ++- .../src/site/markdown/arguments.md | 2 +- .../invoker.properties | 18 +++++++ .../pom.xml | 54 +++++++++++++++++++ .../postbuild.groovy | 35 ++++++++++++ .../test-suppression1.xml | 27 ++++++++++ .../test-suppression2.xml | 27 ++++++++++ .../maven/BaseDependencyCheckMojo.java | 41 +++++++++++--- .../dependencycheck/utils/FileUtils.java | 4 +- .../owasp/dependencycheck/utils/Settings.java | 2 +- 14 files changed, 269 insertions(+), 38 deletions(-) create mode 100644 dependency-check-maven/src/it/730-multiple-suppression-files-configs/invoker.properties create mode 100644 dependency-check-maven/src/it/730-multiple-suppression-files-configs/pom.xml create mode 100644 dependency-check-maven/src/it/730-multiple-suppression-files-configs/postbuild.groovy create mode 100644 dependency-check-maven/src/it/730-multiple-suppression-files-configs/test-suppression1.xml create mode 100644 dependency-check-maven/src/it/730-multiple-suppression-files-configs/test-suppression2.xml diff --git a/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java b/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java index 7588f2105..9761effd3 100644 --- a/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java +++ b/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java @@ -64,7 +64,7 @@ public class Check extends Update { * Whether or not the NSP Analyzer is enabled. */ private Boolean nspAnalyzerEnabled; - + /** * Whether or not the Ruby Bundle Audit Analyzer is enabled. */ @@ -154,6 +154,10 @@ public class Check extends Update { * Default is HTML. */ private String reportFormat = "HTML"; + /** + * Suppression file path. + */ + private String suppressionFile = null; /** * Suppression file paths. */ @@ -462,11 +466,10 @@ public class Check extends Update { * Set the value of suppressionFile. * * @param suppressionFile new value of suppressionFile - * @deprecated property form of suppressionFile has been replaced by a child element */ - @Deprecated public void setSuppressionFile(String suppressionFile) { - throw new BuildException("Definition of a suppression file via a property has been deprecated. Suppression files are now defined as a nested element, please update your configuration."); + this.suppressionFile = suppressionFile; + suppressionFiles.add(suppressionFile); } /** @@ -758,6 +761,7 @@ public class Check extends Update { public void setNodeAnalyzerEnabled(Boolean nodeAnalyzerEnabled) { this.nodeAnalyzerEnabled = nodeAnalyzerEnabled; } + /** * Get the value of nspAnalyzerEnabled. * @@ -766,6 +770,7 @@ public class Check extends Update { public Boolean isNspAnalyzerEnabled() { return nspAnalyzerEnabled; } + /** * Set the value of nspAnalyzerEnabled. * diff --git a/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/SuppressionFile.java b/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/SuppressionFile.java index 962c09cad..0cc3c3364 100644 --- a/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/SuppressionFile.java +++ b/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/SuppressionFile.java @@ -18,8 +18,10 @@ package org.owasp.dependencycheck.taskdefs; /** - * Class : {@link SuppressionFile} - * Responsibility : Models a suppression file nested XML element where the simple content is its location. + * Class : {@link SuppressionFile} Responsibility : Models a suppression file + * nested XML element where the simple content is its location. + * + * @author Phillip Whittlesea */ public class SuppressionFile { diff --git a/dependency-check-ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskTest.java b/dependency-check-ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskTest.java index e1c41c9e8..1e86afed6 100644 --- a/dependency-check-ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskTest.java +++ b/dependency-check-ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskTest.java @@ -127,22 +127,41 @@ public class DependencyCheckTaskTest { buildFileRule.executeTarget(antTaskName); // THEN the ant task executed without error - final File report = new File("target/dependency-check-report.html"); + final File report = new File("target/suppression-report.html"); assertTrue("Expected the DependencyCheck report to be generated", report.exists()); } /** - * Test the DependencyCheckTask deprecated suppression property throws an exception with a warning. + * Test the DependencyCheckTask deprecated suppression property throws an + * exception with a warning. */ @Test - public void testDeprecatedSuppressingCVE() { + public void testSuppressingSingle() { // GIVEN an ant task with a vulnerability using the legacy property - final String antTaskName = "deprecated-suppression"; - + final String antTaskName = "suppression-single"; + // WHEN executing the ant task - // THEN an exception with a warning is thrown - expectedException.expect(BuildException.class); - expectedException.expectMessage("Definition of a suppression file via a property has been deprecated. Suppression files are now defined as a nested element, please update your configuration."); buildFileRule.executeTarget(antTaskName); + + // THEN the ant task executed without error + final File report = new File("target/suppression-single-report.html"); + assertTrue("Expected the DependencyCheck report to be generated", report.exists()); + } + + /** + * Test the DependencyCheckTask deprecated suppression property throws an + * exception with a warning. + */ + @Test + public void testSuppressingMultiple() { + // GIVEN an ant task with a vulnerability using multiple was to configure the suppression file + final String antTaskName = "suppression-multiple"; + + // WHEN executing the ant task + buildFileRule.executeTarget(antTaskName); + + // THEN the ant task executed without error + final File report = new File("target/suppression-multiple-report.html"); + assertTrue("Expected the DependencyCheck report to be generated", report.exists()); } } diff --git a/dependency-check-ant/src/test/resources/build.xml b/dependency-check-ant/src/test/resources/build.xml index 21f7a84a0..68a71e62d 100644 --- a/dependency-check-ant/src/test/resources/build.xml +++ b/dependency-check-ant/src/test/resources/build.xml @@ -72,19 +72,10 @@ - - - - ${project.build.directory}/test-classes/test-suppression1.xml @@ -97,4 +88,30 @@ files="jetty-6.1.0.jar,org.mortbay.jetty.jar"/> + + + + + + + + + + ${project.build.directory}/test-classes/test-suppression2.xml + + + + + + diff --git a/dependency-check-cli/src/main/java/org/owasp/dependencycheck/CliParser.java b/dependency-check-cli/src/main/java/org/owasp/dependencycheck/CliParser.java index aed026bd1..8df1725dd 100644 --- a/dependency-check-cli/src/main/java/org/owasp/dependencycheck/CliParser.java +++ b/dependency-check-cli/src/main/java/org/owasp/dependencycheck/CliParser.java @@ -274,7 +274,8 @@ public final class CliParser { .build(); final Option suppressionFile = Option.builder().argName("file").hasArgs().longOpt(ARGUMENT.SUPPRESSION_FILES) - .desc("The file path to the suppression XML file.") + .desc("The file path to the suppression XML file. This can be specified more then once to utilize multiple " + + "suppression files") .build(); final Option hintsFile = Option.builder().argName("file").hasArg().longOpt(ARGUMENT.HINTS_FILE) @@ -735,7 +736,8 @@ public final class CliParser { public boolean isNodeJsDisabled() { return hasDisableOption(ARGUMENT.DISABLE_NODE_JS, Settings.KEYS.ANALYZER_NODE_PACKAGE_ENABLED); } -/** + + /** * Returns true if the disableNSP command line argument was specified. * * @return true if the disableNSP command line argument was specified; diff --git a/dependency-check-cli/src/site/markdown/arguments.md b/dependency-check-cli/src/site/markdown/arguments.md index bdf2b3826..f418e0595 100644 --- a/dependency-check-cli/src/site/markdown/arguments.md +++ b/dependency-check-cli/src/site/markdown/arguments.md @@ -14,7 +14,7 @@ Short | Argument Name   | Parameter | Description | Requir | \-\-failOnCvss | \ | If the score set between 0 and 10 the exit code from dependency-check will indicate if a vulnerability with a CVSS score equal to or higher was identified. | Optional \-l | \-\-log | \ | The file path to write verbose logging information. | Optional \-n | \-\-noupdate | | Disables the automatic updating of the CPE data. | Optional - | \-\-suppression | \ | The file paths to the suppression XML files; used to suppress [false positives](../general/suppression.html). | Optional + | \-\-suppression | \ | The file paths to the suppression XML files; used to suppress [false positives](../general/suppression.html). This can be specified more then once to utilize multiple suppression files. | Optional \-h | \-\-help | | Print the help message. | Optional | \-\-advancedHelp | | Print the advanced help message. | Optional \-v | \-\-version | | Print the version information. | Optional diff --git a/dependency-check-maven/src/it/730-multiple-suppression-files-configs/invoker.properties b/dependency-check-maven/src/it/730-multiple-suppression-files-configs/invoker.properties new file mode 100644 index 000000000..9c2542d23 --- /dev/null +++ b/dependency-check-maven/src/it/730-multiple-suppression-files-configs/invoker.properties @@ -0,0 +1,18 @@ +# +# This file is part of dependency-check-core. +# +# 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. +# +# Copyright (c) 2017 The OWASP Foundation. All Rights Reserved. +# +invoker.goals = install ${project.groupId}:${project.artifactId}:${project.version}:check diff --git a/dependency-check-maven/src/it/730-multiple-suppression-files-configs/pom.xml b/dependency-check-maven/src/it/730-multiple-suppression-files-configs/pom.xml new file mode 100644 index 000000000..cec15d819 --- /dev/null +++ b/dependency-check-maven/src/it/730-multiple-suppression-files-configs/pom.xml @@ -0,0 +1,54 @@ + + + + 4.0.0 + org.owasp.test + test-multiple-suppression-files + 1.0.0-SNAPSHOT + jar + + + + + com.vaadin.external.google + android-json + 0.0.20131108.vaadin1 + + + com.fasterxml.jackson.dataformat + jackson-dataformat-xml + 2.4.5 + + + + + + + org.owasp + dependency-check-maven + + ${project.basedir}/test-suppression1.xml + + ${project.basedir}/test-suppression2.xml + + + + + + diff --git a/dependency-check-maven/src/it/730-multiple-suppression-files-configs/postbuild.groovy b/dependency-check-maven/src/it/730-multiple-suppression-files-configs/postbuild.groovy new file mode 100644 index 000000000..2a42192d9 --- /dev/null +++ b/dependency-check-maven/src/it/730-multiple-suppression-files-configs/postbuild.groovy @@ -0,0 +1,35 @@ +/* + * This file is part of dependency-check-core. + * + * 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. + * + * Copyright (c) 2017 The OWASP Foundation. All Rights Reserved. + */ + +import org.apache.commons.io.FileUtils +import org.apache.commons.lang.StringUtils + +import java.nio.charset.Charset + +// Check that suppression worked. +String log = FileUtils.readFileToString(new File(basedir, "build.log"), Charset.defaultCharset().name()); +int count = StringUtils.countMatches(log, "CVE-2016-5696"); +if (count > 0) { + System.out.println(String.format("CVE-2016-5696 (android-json-0.0.20131108.vaadin1.jar) was identified and should be suppressed")); + return false; +} +count = StringUtils.countMatches(log, "CVE-2016-7051"); +if (count > 0) { + System.out.println(String.format("CVE-2016-7051 (jackson-module-jaxb-annotations-2.4.5.jar) was identified and should be suppressed")); + return false; +} diff --git a/dependency-check-maven/src/it/730-multiple-suppression-files-configs/test-suppression1.xml b/dependency-check-maven/src/it/730-multiple-suppression-files-configs/test-suppression1.xml new file mode 100644 index 000000000..e3bd8c0b0 --- /dev/null +++ b/dependency-check-maven/src/it/730-multiple-suppression-files-configs/test-suppression1.xml @@ -0,0 +1,27 @@ + + + + + + ^com\.vaadin\.external\.google:android-json:.*$ + cpe:/a:google:android + + diff --git a/dependency-check-maven/src/it/730-multiple-suppression-files-configs/test-suppression2.xml b/dependency-check-maven/src/it/730-multiple-suppression-files-configs/test-suppression2.xml new file mode 100644 index 000000000..f0b4fad7b --- /dev/null +++ b/dependency-check-maven/src/it/730-multiple-suppression-files-configs/test-suppression2.xml @@ -0,0 +1,27 @@ + + + + + + ^com\.fasterxml\.jackson.*:.*:.*$ + cpe:/a:fasterxml:jackson + + diff --git a/dependency-check-maven/src/main/java/org/owasp/dependencycheck/maven/BaseDependencyCheckMojo.java b/dependency-check-maven/src/main/java/org/owasp/dependencycheck/maven/BaseDependencyCheckMojo.java index 3b08fe763..941864d3e 100644 --- a/dependency-check-maven/src/main/java/org/owasp/dependencycheck/maven/BaseDependencyCheckMojo.java +++ b/dependency-check-maven/src/main/java/org/owasp/dependencycheck/maven/BaseDependencyCheckMojo.java @@ -21,6 +21,7 @@ import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; +import java.util.Arrays; import java.util.List; import java.util.Locale; import org.apache.maven.artifact.Artifact; @@ -203,7 +204,11 @@ public abstract class BaseDependencyCheckMojo extends AbstractMojo implements Ma */ @Parameter(required = false) private String[] suppressionFiles; - + /** + * The paths to the suppression file. + */ + @Parameter(required = false) + private String suppressionFile; /** * The path to the hints file. */ @@ -415,7 +420,8 @@ public abstract class BaseDependencyCheckMojo extends AbstractMojo implements Ma private boolean skipSystemScope = false; /** - * Skip analysis for dependencies which type matches this regular expression. + * Skip analysis for dependencies which type matches this regular + * expression. */ @SuppressWarnings("CanBeFinal") @Parameter(property = "skipArtifactType", required = false) @@ -488,7 +494,6 @@ public abstract class BaseDependencyCheckMojo extends AbstractMojo implements Ma */ private Filter artifactTypeExcluded; - // // /** @@ -660,8 +665,8 @@ public abstract class BaseDependencyCheckMojo extends AbstractMojo implements Ma List nodes, ProjectBuildingRequest buildingRequest) { ExceptionCollection exCol = null; for (DependencyNode dependencyNode : nodes) { - if (artifactScopeExcluded.passes(dependencyNode.getArtifact().getScope()) || - artifactTypeExcluded.passes(dependencyNode.getArtifact().getType())) { + if (artifactScopeExcluded.passes(dependencyNode.getArtifact().getScope()) + || artifactTypeExcluded.passes(dependencyNode.getArtifact().getType())) { continue; } exCol = collectDependencies(engine, project, dependencyNode.getChildren(), buildingRequest); @@ -686,7 +691,8 @@ public abstract class BaseDependencyCheckMojo extends AbstractMojo implements Ma } if (!isResolved) { getLog().error("Unable to resolve system scoped dependency: " + dependencyNode.toNodeString()); - exCol.addException(new DependencyNotFoundException("Unable to resolve system scoped dependency: " + dependencyNode.toNodeString())); + exCol.addException(new DependencyNotFoundException("Unable to resolve system scoped dependency: " + + dependencyNode.toNodeString())); } } else { final ArtifactCoordinate coordinate = TransferUtils.toArtifactCoordinate(dependencyNode.getArtifact()); @@ -924,8 +930,8 @@ public abstract class BaseDependencyCheckMojo extends AbstractMojo implements Ma Settings.setStringIfNotNull(Settings.KEYS.PROXY_PASSWORD, password); Settings.setStringIfNotNull(Settings.KEYS.PROXY_NON_PROXY_HOSTS, proxy.getNonProxyHosts()); } - - Settings.setArrayIfNotEmpty(Settings.KEYS.SUPPRESSION_FILE, suppressionFiles); + final String[] suppressions = determineSuppressions(); + Settings.setArrayIfNotEmpty(Settings.KEYS.SUPPRESSION_FILE, suppressions); Settings.setStringIfNotEmpty(Settings.KEYS.CONNECTION_TIMEOUT, connectionTimeout); Settings.setStringIfNotEmpty(Settings.KEYS.HINTS_FILE, hintsFile); @@ -1015,6 +1021,25 @@ public abstract class BaseDependencyCheckMojo extends AbstractMojo implements Ma artifactTypeExcluded = new ArtifactTypeExcluded(skipArtifactType); } + /** + * Combines the configured suppressionFile and suppressionFiles into a + * single array. + * + * @return an array of suppression file paths + */ + private String[] determineSuppressions() { + String[] suppressions = suppressionFiles; + if (suppressionFile != null) { + if (suppressions == null) { + suppressions = new String[]{suppressionFile}; + } else { + suppressions = Arrays.copyOf(suppressions, suppressions.length + 1); + suppressions[suppressions.length - 1] = suppressionFile; + } + } + return suppressions; + } + /** * Returns the maven proxy. * diff --git a/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/FileUtils.java b/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/FileUtils.java index b4a898192..50df24f14 100644 --- a/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/FileUtils.java +++ b/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/FileUtils.java @@ -151,10 +151,10 @@ public final class FileUtils { } /** - * Gets the {@link InputStream} for this resource + * Gets the {@link InputStream} for this resource. * * @param resource path - * @return + * @return the input stream for the given resource */ public static InputStream getResourceAsStream(String resource) { return FileUtils.class.getClassLoader() != null diff --git a/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java b/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java index f84ff734f..cb4eece09 100644 --- a/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java +++ b/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java @@ -763,7 +763,7 @@ public final class Settings { private static File getJarPath() { String decodedPath = "."; String jarPath = ""; - ProtectionDomain domain = Settings.class.getProtectionDomain(); + final ProtectionDomain domain = Settings.class.getProtectionDomain(); if (domain != null && domain.getCodeSource() != null && domain.getCodeSource().getLocation() != null) { jarPath = Settings.class.getProtectionDomain().getCodeSource().getLocation().getPath(); }