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. */