From b0f9935fcbfba23c164f7d41006cb9da83da2e8e Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sat, 20 May 2017 07:37:46 -0400 Subject: [PATCH] updated to resolve issue #696 --- .../java/org/owasp/dependencycheck/App.java | 13 +-- .../org/owasp/dependencycheck/CliParser.java | 72 +++++++++----- .../org/owasp/dependencycheck/AppTest.java | 96 +++++++++++++++++-- .../src/test/resources/sample.properties | 33 +++++++ .../src/test/resources/sample2.properties | 33 +++++++ pom.xml | 1 + 6 files changed, 209 insertions(+), 39 deletions(-) create mode 100644 dependency-check-cli/src/test/resources/sample.properties create mode 100644 dependency-check-cli/src/test/resources/sample2.properties 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 52c6fce77..399dd15b3 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 @@ -353,8 +353,7 @@ public class App { * @throws InvalidSettingException thrown when a user defined properties * file is unable to be loaded. */ - private void populateSettings(CliParser cli) throws InvalidSettingException { - final boolean autoUpdate = cli.isAutoUpdate(); + protected void populateSettings(CliParser cli) throws InvalidSettingException { final String connectionTimeout = cli.getConnectionTimeout(); final String proxyServer = cli.getProxyServer(); final String proxyPort = cli.getProxyPort(); @@ -377,7 +376,8 @@ public class App { final String cveBase12 = cli.getBaseCve12Url(); final String cveBase20 = cli.getBaseCve20Url(); final Integer cveValidForHours = cli.getCveValidForHours(); - final boolean experimentalEnabled = cli.isExperimentalEnabled(); + final Boolean autoUpdate = cli.isAutoUpdate(); + final Boolean experimentalEnabled = cli.isExperimentalEnabled(); if (propertiesFile != null) { try { @@ -390,7 +390,7 @@ public class App { } // We have to wait until we've merged the properties before attempting to set whether we use // the proxy for Nexus since it could be disabled in the properties, but not explicitly stated - // on the command line + // on the command line. This is true of other boolean values set below not using the setBooleanIfNotNull. final boolean nexusUsesProxy = cli.isNexusUsesProxy(); if (dataDirectory != null) { Settings.setString(Settings.KEYS.DATA_DIRECTORY, dataDirectory); @@ -404,7 +404,7 @@ public class App { final File dataDir = new File(base, sub); Settings.setString(Settings.KEYS.DATA_DIRECTORY, dataDir.getAbsolutePath()); } - Settings.setBoolean(Settings.KEYS.AUTO_UPDATE, autoUpdate); + Settings.setBooleanIfNotNull(Settings.KEYS.AUTO_UPDATE, autoUpdate); Settings.setStringIfNotEmpty(Settings.KEYS.PROXY_SERVER, proxyServer); Settings.setStringIfNotEmpty(Settings.KEYS.PROXY_PORT, proxyPort); Settings.setStringIfNotEmpty(Settings.KEYS.PROXY_USERNAME, proxyUser); @@ -415,7 +415,8 @@ public class App { Settings.setIntIfNotNull(Settings.KEYS.CVE_CHECK_VALID_FOR_HOURS, cveValidForHours); //File Type Analyzer Settings - Settings.setBoolean(Settings.KEYS.ANALYZER_EXPERIMENTAL_ENABLED, experimentalEnabled); + Settings.setBooleanIfNotNull(Settings.KEYS.ANALYZER_EXPERIMENTAL_ENABLED, experimentalEnabled); + Settings.setBoolean(Settings.KEYS.ANALYZER_JAR_ENABLED, !cli.isJarDisabled()); Settings.setBoolean(Settings.KEYS.ANALYZER_ARCHIVE_ENABLED, !cli.isArchiveDisabled()); Settings.setBoolean(Settings.KEYS.ANALYZER_PYTHON_DISTRIBUTION_ENABLED, !cli.isPythonDistributionDisabled()); 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 c259e50d0..2ac6152c6 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 @@ -567,6 +567,32 @@ public final class CliParser { return value; } + /** + * Utility method to determine if one of the disable options has been set. + * If not set, this method will check the currently configured settings for + * the current value to return. + * + * Example given `--disableArchive` on the command line would cause this + * method to return true for the disable archive setting. + * + * @param argument the command line argument + * @param setting the corresponding settings key + * @return true if the disable option was set, if not set the currently + * configured value will be returned + */ + private boolean hasDisableOption(String argument, String setting) { + if (line == null || !line.hasOption(argument)) { + try { + return !Settings.getBoolean(setting); + } catch (InvalidSettingException ise) { + LOGGER.warn("Invalid property setting '{}' defaulting to false", setting); + return false; + } + } else { + return true; + } + } + /** * Returns true if the disableJar command line argument was specified. * @@ -574,7 +600,7 @@ public final class CliParser { * otherwise false */ public boolean isJarDisabled() { - return (line != null) && line.hasOption(ARGUMENT.DISABLE_JAR); + return hasDisableOption(ARGUMENT.DISABLE_JAR, Settings.KEYS.ANALYZER_JAR_ENABLED); } /** @@ -584,7 +610,7 @@ public final class CliParser { * otherwise false */ public boolean isArchiveDisabled() { - return (line != null) && line.hasOption(ARGUMENT.DISABLE_ARCHIVE); + return hasDisableOption(ARGUMENT.DISABLE_ARCHIVE, Settings.KEYS.ANALYZER_ARCHIVE_ENABLED); } /** @@ -594,7 +620,7 @@ public final class CliParser { * otherwise false */ public boolean isNuspecDisabled() { - return (line != null) && line.hasOption(ARGUMENT.DISABLE_NUSPEC); + return hasDisableOption(ARGUMENT.DISABLE_NUSPEC, Settings.KEYS.ANALYZER_NUSPEC_ENABLED); } /** @@ -604,7 +630,7 @@ public final class CliParser { * otherwise false */ public boolean isAssemblyDisabled() { - return (line != null) && line.hasOption(ARGUMENT.DISABLE_ASSEMBLY); + return hasDisableOption(ARGUMENT.DISABLE_ASSEMBLY, Settings.KEYS.ANALYZER_ASSEMBLY_ENABLED); } /** @@ -615,7 +641,7 @@ public final class CliParser { * specified; otherwise false */ public boolean isBundleAuditDisabled() { - return (line != null) && line.hasOption(ARGUMENT.DISABLE_BUNDLE_AUDIT); + return hasDisableOption(ARGUMENT.DISABLE_BUNDLE_AUDIT, Settings.KEYS.ANALYZER_BUNDLE_AUDIT_ENABLED); } /** @@ -625,7 +651,7 @@ public final class CliParser { * otherwise false */ public boolean isPythonDistributionDisabled() { - return (line != null) && line.hasOption(ARGUMENT.DISABLE_PY_DIST); + return hasDisableOption(ARGUMENT.DISABLE_PY_DIST, Settings.KEYS.ANALYZER_PYTHON_DISTRIBUTION_ENABLED); } /** @@ -635,7 +661,7 @@ public final class CliParser { * otherwise false */ public boolean isPythonPackageDisabled() { - return (line != null) && line.hasOption(ARGUMENT.DISABLE_PY_PKG); + return hasDisableOption(ARGUMENT.DISABLE_PY_PKG, Settings.KEYS.ANALYZER_PYTHON_PACKAGE_ENABLED); } /** @@ -645,7 +671,7 @@ public final class CliParser { * argument was specified; otherwise false */ public boolean isRubyGemspecDisabled() { - return (null != line) && line.hasOption(ARGUMENT.DISABLE_RUBYGEMS); + return hasDisableOption(ARGUMENT.DISABLE_RUBYGEMS, Settings.KEYS.ANALYZER_RUBY_GEMSPEC_ENABLED); } /** @@ -655,7 +681,7 @@ public final class CliParser { * otherwise false */ public boolean isCmakeDisabled() { - return (line != null) && line.hasOption(ARGUMENT.DISABLE_CMAKE); + return hasDisableOption(ARGUMENT.DISABLE_CMAKE, Settings.KEYS.ANALYZER_CMAKE_ENABLED); } /** @@ -665,7 +691,7 @@ public final class CliParser { * otherwise false */ public boolean isAutoconfDisabled() { - return (line != null) && line.hasOption(ARGUMENT.DISABLE_AUTOCONF); + return hasDisableOption(ARGUMENT.DISABLE_AUTOCONF, Settings.KEYS.ANALYZER_AUTOCONF_ENABLED); } /** @@ -675,7 +701,7 @@ public final class CliParser { * otherwise false */ public boolean isComposerDisabled() { - return (line != null) && line.hasOption(ARGUMENT.DISABLE_COMPOSER); + return hasDisableOption(ARGUMENT.DISABLE_COMPOSER, Settings.KEYS.ANALYZER_COMPOSER_LOCK_ENABLED); } /** @@ -685,7 +711,7 @@ public final class CliParser { * otherwise false */ public boolean isNexusDisabled() { - return (line != null) && line.hasOption(ARGUMENT.DISABLE_NEXUS); + return hasDisableOption(ARGUMENT.DISABLE_NEXUS, Settings.KEYS.ANALYZER_NEXUS_ENABLED); } /** @@ -695,7 +721,7 @@ public final class CliParser { * otherwise false */ public boolean isOpenSSLDisabled() { - return (line != null) && line.hasOption(ARGUMENT.DISABLE_OPENSSL); + return hasDisableOption(ARGUMENT.DISABLE_OPENSSL, Settings.KEYS.ANALYZER_OPENSSL_ENABLED); } /** @@ -705,7 +731,7 @@ public final class CliParser { * otherwise false */ public boolean isNodeJsDisabled() { - return (line != null) && line.hasOption(ARGUMENT.DISABLE_NODE_JS); + return hasDisableOption(ARGUMENT.DISABLE_NODE_JS, Settings.KEYS.ANALYZER_NODE_PACKAGE_ENABLED); } /** @@ -716,7 +742,7 @@ public final class CliParser { * specified; otherwise false */ public boolean isCocoapodsAnalyzerDisabled() { - return (line != null) && line.hasOption(ARGUMENT.DISABLE_COCOAPODS); + return hasDisableOption(ARGUMENT.DISABLE_COCOAPODS, Settings.KEYS.ANALYZER_COCOAPODS_ENABLED); } /** @@ -727,7 +753,7 @@ public final class CliParser { * argument was specified; otherwise false */ public boolean isSwiftPackageAnalyzerDisabled() { - return (line != null) && line.hasOption(ARGUMENT.DISABLE_SWIFT); + return hasDisableOption(ARGUMENT.DISABLE_SWIFT, Settings.KEYS.ANALYZER_SWIFT_PACKAGE_MANAGER_ENABLED); } /** @@ -737,7 +763,7 @@ public final class CliParser { * otherwise false */ public boolean isCentralDisabled() { - return (line != null) && line.hasOption(ARGUMENT.DISABLE_CENTRAL); + return hasDisableOption(ARGUMENT.DISABLE_CENTRAL, Settings.KEYS.ANALYZER_CENTRAL_ENABLED); } /** @@ -1029,10 +1055,10 @@ public final class CliParser { * disabled via the command line this will return false. * * @return true if auto-update is allowed; otherwise - * false + * null */ - public boolean isAutoUpdate() { - return line != null && !line.hasOption(ARGUMENT.DISABLE_AUTO_UPDATE); + public Boolean isAutoUpdate() { + return (line != null && line.hasOption(ARGUMENT.DISABLE_AUTO_UPDATE)) ? false : null; } /** @@ -1134,10 +1160,10 @@ public final class CliParser { /** * Returns true if the experimental analyzers are enabled. * - * @return true if the experimental analyzers are enabled; otherwise false + * @return true if the experimental analyzers are enabled; otherwise null */ - public boolean isExperimentalEnabled() { - return line.hasOption(ARGUMENT.EXPERIMENTAL); + public Boolean isExperimentalEnabled() { + return (line != null && line.hasOption(ARGUMENT.EXPERIMENTAL)) ? true : null; } /** 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 fdadb0e39..e6569e761 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,18 +13,28 @@ * See the License for the specific language governing permissions and * limitations under the License. * - * Copyright (c) 2015 The OWASP Foundatio. All Rights Reserved. + * Copyright (c) 2017 The OWASP Foundatio. All Rights Reserved. */ package org.owasp.dependencycheck; +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.Test; import static org.junit.Assert.*; +import org.owasp.dependencycheck.utils.InvalidSettingException; +import org.owasp.dependencycheck.utils.Settings; /** * * @author jeremy */ public class AppTest { + /** * Test of ensureCanonicalPath method, of class App. */ @@ -35,17 +45,83 @@ public class AppTest { String result = instance.ensureCanonicalPath(file); assertFalse(result.contains("..")); assertTrue(result.endsWith("*.jar")); - } - /** - * Test of ensureCanonicalPath method, of class App. - */ - @Test - public void testEnsureCanonicalPath2() { - String file = "../some/skip/../path/file.txt"; - App instance = new App(); + file = "../some/skip/../path/file.txt"; String expResult = "/some/path/file.txt"; - String result = instance.ensureCanonicalPath(file); + result = instance.ensureCanonicalPath(file); 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)); + } + + @Test + public void testPopulateSettings() throws FileNotFoundException, ParseException, InvalidSettingException, URISyntaxException { + File prop = new File(this.getClass().getClassLoader().getResource("sample.properties").toURI().getPath()); + 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"}; + expected.put(Settings.KEYS.AUTO_UPDATE, Boolean.FALSE); + expected.put(Settings.KEYS.ANALYZER_ARCHIVE_ENABLED, Boolean.TRUE); + assertTrue(testBooleanProperties(args2, expected)); + + 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"}; + 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"}; + 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"}; + 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"}; + expected.put(Settings.KEYS.AUTO_UPDATE, Boolean.FALSE); + expected.put(Settings.KEYS.ANALYZER_ARCHIVE_ENABLED, Boolean.TRUE); + assertTrue(testBooleanProperties(args7, expected)); + + 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)); + + + } + + private boolean testBooleanProperties(String[] args, Map expected) throws URISyntaxException, FileNotFoundException, ParseException, InvalidSettingException { + Settings.initialize(); + try { + final CliParser cli = new CliParser(); + cli.parse(args); + App instance = new App(); + instance.populateSettings(cli); + boolean results = true; + for (Map.Entry entry : expected.entrySet()) { + results &= Settings.getBoolean(entry.getKey()) == entry.getValue(); + } + + return results; + } finally { + Settings.cleanup(); + } + } } diff --git a/dependency-check-cli/src/test/resources/sample.properties b/dependency-check-cli/src/test/resources/sample.properties new file mode 100644 index 000000000..0b45d5d04 --- /dev/null +++ b/dependency-check-cli/src/test/resources/sample.properties @@ -0,0 +1,33 @@ +autoupdate=false + +analyzer.experimental.enabled=false +analyzer.jar.enabled=true +analyzer.archive.enabled=true +analyzer.node.package.enabled=true +analyzer.composer.lock.enabled=true +analyzer.python.distribution.enabled=true +analyzer.python.package.enabled=true +analyzer.ruby.gemspec.enabled=true +analyzer.autoconf.enabled=true +analyzer.cmake.enabled=true +analyzer.assembly.enabled=true +analyzer.nuspec.enabled=true +analyzer.openssl.enabled=true +analyzer.central.enabled=true +analyzer.nexus.enabled=false +analyzer.cocoapods.enabled=true +analyzer.swift.package.manager.enabled=true +#whether the nexus analyzer uses the proxy +analyzer.nexus.proxy=true +analyzer.cpe.enabled=true +analyzer.cpesuppression.enabled=true +analyzer.dependencybundling.enabled=true +analyzer.dependencymerging.enabled=true +analyzer.falsepositive.enabled=true +analyzer.filename.enabled=true +analyzer.hint.enabled=true +analyzer.nvdcve.enabled=true +analyzer.vulnerabilitysuppression.enabled=true +updater.nvdcve.enabled=true +updater.versioncheck.enabled=true +analyzer.versionfilter.enabled=true \ No newline at end of file diff --git a/dependency-check-cli/src/test/resources/sample2.properties b/dependency-check-cli/src/test/resources/sample2.properties new file mode 100644 index 000000000..34a2efe65 --- /dev/null +++ b/dependency-check-cli/src/test/resources/sample2.properties @@ -0,0 +1,33 @@ +autoupdate=true + +analyzer.experimental.enabled=false +analyzer.jar.enabled=true +analyzer.archive.enabled=true +analyzer.node.package.enabled=true +analyzer.composer.lock.enabled=true +analyzer.python.distribution.enabled=true +analyzer.python.package.enabled=true +analyzer.ruby.gemspec.enabled=true +analyzer.autoconf.enabled=true +analyzer.cmake.enabled=true +analyzer.assembly.enabled=true +analyzer.nuspec.enabled=true +analyzer.openssl.enabled=true +analyzer.central.enabled=true +analyzer.nexus.enabled=false +analyzer.cocoapods.enabled=true +analyzer.swift.package.manager.enabled=true +#whether the nexus analyzer uses the proxy +analyzer.nexus.proxy=true +analyzer.cpe.enabled=true +analyzer.cpesuppression.enabled=true +analyzer.dependencybundling.enabled=true +analyzer.dependencymerging.enabled=true +analyzer.falsepositive.enabled=true +analyzer.filename.enabled=true +analyzer.hint.enabled=true +analyzer.nvdcve.enabled=true +analyzer.vulnerabilitysuppression.enabled=true +updater.nvdcve.enabled=true +updater.versioncheck.enabled=true +analyzer.versionfilter.enabled=true \ No newline at end of file diff --git a/pom.xml b/pom.xml index 370e4c9fd..f49db6716 100644 --- a/pom.xml +++ b/pom.xml @@ -134,6 +134,7 @@ Copyright (c) 2012 - Jeremy Long 3.0 2.17 3.6 +