From 08d7657ac23de6926feacb137dcdad5d9b7a2f7e Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Wed, 30 Oct 2024 05:19:27 -0400 Subject: [PATCH] feat: PHP Composer Analyzer Scans packages-dev by default (#7114) --- .../owasp/dependencycheck/taskdefs/Check.java | 23 +++++++ ant/src/site/markdown/configuration.md | 1 + .../java/org/owasp/dependencycheck/App.java | 2 + .../org/owasp/dependencycheck/CliParser.java | 5 ++ cli/src/site/markdown/arguments.md | 1 + .../analyzer/ComposerLockAnalyzer.java | 3 +- .../data/composer/ComposerLockParser.java | 63 ++++++++++++------- .../data/composer/ComposerLockParserTest.java | 20 ++++-- core/src/test/resources/composer.lock | 5 +- .../maven/BaseDependencyCheckMojo.java | 6 ++ maven/src/site/markdown/configuration.md | 3 +- .../dependency-check-gradle/configuration.md | 1 + .../owasp/dependencycheck/utils/Settings.java | 5 ++ 13 files changed, 106 insertions(+), 32 deletions(-) diff --git a/ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java b/ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java index b08e2d55c51..39e5983564e 100644 --- a/ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java +++ b/ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java @@ -308,6 +308,10 @@ public class Check extends Update { * Whether or not the PHP Composer Analyzer is enabled. */ private Boolean composerAnalyzerEnabled; + /** + * Whether or not the PHP Composer Analyzer will skip "packages-dev". + */ + private Boolean composerAnalyzerSkipDev; /** * Whether or not the Perl CPAN File Analyzer is enabled. */ @@ -943,6 +947,24 @@ public Boolean isComposerAnalyzerEnabled() { public void setComposerAnalyzerEnabled(Boolean composerAnalyzerEnabled) { this.composerAnalyzerEnabled = composerAnalyzerEnabled; } + + /** + * Get the value of composerAnalyzerSkipDev. + * + * @return the value of composerAnalyzerSkipDev + */ + public Boolean isComposerAnalyzerSkipDev() { + return composerAnalyzerSkipDev; + } + + /** + * Set the value of composerAnalyzerSkipDev. + * + * @param composerAnalyzerSkipDev new value of composerAnalyzerSkipDev + */ + public void setComposerAnalyzerSkipDev(Boolean composerAnalyzerSkipDev) { + this.composerAnalyzerSkipDev = composerAnalyzerSkipDev; + } /** * Get the value of cpanfileAnalyzerEnabled. @@ -2183,6 +2205,7 @@ protected void populateSettings() throws BuildException { getSettings().setBooleanIfNotNull(Settings.KEYS.ANALYZER_PIPFILE_ENABLED, pipfileAnalyzerEnabled); getSettings().setBooleanIfNotNull(Settings.KEYS.ANALYZER_POETRY_ENABLED, poetryAnalyzerEnabled); getSettings().setBooleanIfNotNull(Settings.KEYS.ANALYZER_COMPOSER_LOCK_ENABLED, composerAnalyzerEnabled); + getSettings().setBooleanIfNotNull(Settings.KEYS.ANALYZER_COMPOSER_LOCK_SKIP_DEV, composerAnalyzerSkipDev); getSettings().setBooleanIfNotNull(Settings.KEYS.ANALYZER_CPANFILE_ENABLED, cpanfileAnalyzerEnabled); getSettings().setBooleanIfNotNull(Settings.KEYS.ANALYZER_NODE_PACKAGE_ENABLED, nodeAnalyzerEnabled); getSettings().setBooleanIfNotNull(Settings.KEYS.ANALYZER_NODE_PACKAGE_SKIPDEV, nodePackageSkipDevDependencies); diff --git a/ant/src/site/markdown/configuration.md b/ant/src/site/markdown/configuration.md index 2c9d11de8e3..a186890fa86 100644 --- a/ant/src/site/markdown/configuration.md +++ b/ant/src/site/markdown/configuration.md @@ -105,6 +105,7 @@ pipAnalyzerEnabled | Sets whether the [experimental](../analyze pipfileAnalyzerEnabled | Sets whether the [experimental](../analyzers/index.html) Pipfile Analyzer should be used. `enableExperimental` must be set to true. | true poetryAnalyzerEnabled | Sets whether the [experimental](../analyzers/index.html) Poetry Analyzer should be used. `enableExperimental` must be set to true. | true composerAnalyzerEnabled | Sets whether the [experimental](../analyzers/index.html) PHP Composer Lock File Analyzer should be used. `enableExperimental` must be set to true. | true +composerAnalyzerSkipDev | Sets whether the [experimental](../analyzers/index.html) PHP Composer Lock File Analyzer should skip "packages-dev" | false cpanfileAnalyzerEnabled | Sets whether the [experimental](../analyzers/index.html) Perl CPAN File Analyzer should be used. `enableExperimental` must be set to true. | true nodeAnalyzerEnabled | Sets whether the [retired](../analyzers/index.html) Node.js Analyzer should be used. | true nodeAuditAnalyzerEnabled | Sets whether the Node Audit Analyzer should be used. This analyzer requires an internet connection. | true diff --git a/cli/src/main/java/org/owasp/dependencycheck/App.java b/cli/src/main/java/org/owasp/dependencycheck/App.java index a1271cd4691..ff1049ca6f2 100644 --- a/cli/src/main/java/org/owasp/dependencycheck/App.java +++ b/cli/src/main/java/org/owasp/dependencycheck/App.java @@ -553,6 +553,8 @@ protected void populateSettings(CliParser cli) throws InvalidSettingException { !cli.isDisabled(CliParser.ARGUMENT.DISABLE_OPENSSL, Settings.KEYS.ANALYZER_OPENSSL_ENABLED)); settings.setBoolean(Settings.KEYS.ANALYZER_COMPOSER_LOCK_ENABLED, !cli.isDisabled(CliParser.ARGUMENT.DISABLE_COMPOSER, Settings.KEYS.ANALYZER_COMPOSER_LOCK_ENABLED)); + settings.setBooleanIfNotNull(Settings.KEYS.ANALYZER_COMPOSER_LOCK_SKIP_DEV, + cli.hasOption(CliParser.ARGUMENT.COMPOSER_LOCK_SKIP_DEV)); settings.setBoolean(Settings.KEYS.ANALYZER_CPANFILE_ENABLED, !cli.isDisabled(CliParser.ARGUMENT.DISABLE_CPAN, Settings.KEYS.ANALYZER_CPANFILE_ENABLED)); settings.setBoolean(Settings.KEYS.ANALYZER_GOLANG_DEP_ENABLED, diff --git a/cli/src/main/java/org/owasp/dependencycheck/CliParser.java b/cli/src/main/java/org/owasp/dependencycheck/CliParser.java index f5826b12858..46264191fc6 100644 --- a/cli/src/main/java/org/owasp/dependencycheck/CliParser.java +++ b/cli/src/main/java/org/owasp/dependencycheck/CliParser.java @@ -494,6 +494,7 @@ private void addAdvancedOptions(final Options options) { .addOption(newOption(ARGUMENT.DISABLE_PIP, "Disable the pip Analyzer.")) .addOption(newOption(ARGUMENT.DISABLE_PIPFILE, "Disable the Pipfile Analyzer.")) .addOption(newOption(ARGUMENT.DISABLE_COMPOSER, "Disable the PHP Composer Analyzer.")) + .addOption(newOption(ARGUMENT.COMPOSER_LOCK_SKIP_DEV, "Configures the PHP Composer Analyzer to skip packages-dev")) .addOption(newOption(ARGUMENT.DISABLE_CPAN, "Disable the Perl CPAN file Analyzer.")) .addOption(newOption(ARGUMENT.DISABLE_POETRY, "Disable the Poetry Analyzer.")) .addOption(newOption(ARGUMENT.DISABLE_GOLANG_MOD, "Disable the Golang Mod Analyzer.")) @@ -1249,6 +1250,10 @@ public static class ARGUMENT { * Disables the PHP Composer Analyzer. */ public static final String DISABLE_COMPOSER = "disableComposer"; + /** + * Whether the PHP Composer Analyzer skips dev packages. + */ + public static final String COMPOSER_LOCK_SKIP_DEV = "composerSkipDev"; /** * Disables the Perl CPAN File Analyzer. */ diff --git a/cli/src/site/markdown/arguments.md b/cli/src/site/markdown/arguments.md index 24e1c182a33..676020ce393 100644 --- a/cli/src/site/markdown/arguments.md +++ b/cli/src/site/markdown/arguments.md @@ -74,6 +74,7 @@ Advanced Options | | \-\-zipExtensions | \ | A comma-separated list of additional file extensions to be treated like a ZIP file, the contents will be extracted and analyzed. |   | | | \-\-disableJar | | Sets whether the Jar Analyzer will be disabled. |   | | | \-\-disableComposer | | Sets whether the [experimental](../analyzers/index.html) PHP Composer Lock File Analyzer will be disabled. |   | +| | \-\-composerSkipDev | | Sets whether the [experimental](../analyzers/index.html) PHP Composer Lock File Analyzer should skip "packages-dev". |   | | | \-\-disableCpan | | Sets whether the [experimental](../analyzers/index.html) Perl CPAN File Analyzer will be disabled. |   | | | \-\-disableDart | | Sets whether the [experimental](../analyzers/index.html) Dart Analyzer will be disabled. |   | | | \-\-disableOssIndex | | Sets whether the [OSS Index Analyzer](../analyzers/oss-index-analyzer.html) will be disabled. This analyzer requires an internet connection. |   | diff --git a/core/src/main/java/org/owasp/dependencycheck/analyzer/ComposerLockAnalyzer.java b/core/src/main/java/org/owasp/dependencycheck/analyzer/ComposerLockAnalyzer.java index 99f2c200dbf..3970c58d1c9 100644 --- a/core/src/main/java/org/owasp/dependencycheck/analyzer/ComposerLockAnalyzer.java +++ b/core/src/main/java/org/owasp/dependencycheck/analyzer/ComposerLockAnalyzer.java @@ -108,7 +108,8 @@ protected void prepareFileTypeAnalyzer(Engine engine) throws InitializationExcep protected void analyzeDependency(Dependency dependency, Engine engine) throws AnalysisException { engine.removeDependency(dependency); try (FileInputStream fis = new FileInputStream(dependency.getActualFile())) { - final ComposerLockParser clp = new ComposerLockParser(fis); + final boolean skipdev = getSettings().getBoolean(Settings.KEYS.ANALYZER_COMPOSER_LOCK_SKIP_DEV, false); + final ComposerLockParser clp = new ComposerLockParser(fis, skipdev); LOGGER.debug("Checking composer.lock file {}", dependency.getActualFilePath()); clp.process(); clp.getDependencies().stream().map((dep) -> { diff --git a/core/src/main/java/org/owasp/dependencycheck/data/composer/ComposerLockParser.java b/core/src/main/java/org/owasp/dependencycheck/data/composer/ComposerLockParser.java index e3b4bae65f5..c4ceac7cc7b 100644 --- a/core/src/main/java/org/owasp/dependencycheck/data/composer/ComposerLockParser.java +++ b/core/src/main/java/org/owasp/dependencycheck/data/composer/ComposerLockParser.java @@ -32,7 +32,8 @@ import javax.annotation.concurrent.NotThreadSafe; /** - * Parses a Composer.lock file from an input stream. In a separate class so it can hopefully be injected. + * Parses a Composer.lock file from an input stream. In a separate class so it + * can hopefully be injected. * * @author colezlaw */ @@ -43,12 +44,14 @@ public class ComposerLockParser { * The JsonReader for parsing JSON */ private final JsonReader jsonReader; - /** * The List of ComposerDependencies found */ private final List composerDependencies; - + /** + * Whether to skip dev dependencies. + */ + private final boolean skipDev; /** * The LOGGER */ @@ -59,10 +62,11 @@ public class ComposerLockParser { * * @param inputStream the InputStream to parse */ - public ComposerLockParser(InputStream inputStream) { + public ComposerLockParser(InputStream inputStream, boolean skipDev) { LOGGER.debug("Creating a ComposerLockParser"); this.jsonReader = Json.createReader(inputStream); this.composerDependencies = new ArrayList<>(); + this.skipDev = skipDev; } /** @@ -76,26 +80,14 @@ public void process() { LOGGER.debug("Found packages"); final JsonArray packages = composer.getJsonArray("packages"); for (JsonObject pkg : packages.getValuesAs(JsonObject.class)) { - if (pkg.containsKey("name")) { - final String groupName = pkg.getString("name"); - if (groupName.indexOf('/') >= 0 && groupName.indexOf('/') <= groupName.length() - 1) { - if (pkg.containsKey("version")) { - final String group = groupName.substring(0, groupName.indexOf('/')); - final String project = groupName.substring(groupName.indexOf('/') + 1); - String version = pkg.getString("version"); - // Some version numbers begin with v - which doesn't end up matching CPE's - if (version.startsWith("v")) { - version = version.substring(1); - } - LOGGER.debug("Got package {}/{}/{}", group, project, version); - composerDependencies.add(new ComposerDependency(group, project, version)); - } else { - LOGGER.debug("Group/package {} does not have a version", groupName); - } - } else { - LOGGER.debug("Got a dependency with no name"); - } - } + processPackageEntry(pkg); + } + } + if (composer.containsKey("packages-dev") && !skipDev) { + LOGGER.debug("Found packages-dev"); + final JsonArray devPackages = composer.getJsonArray("packages-dev"); + for (JsonObject pkg : devPackages.getValuesAs(JsonObject.class)) { + processPackageEntry(pkg); } } } catch (JsonParsingException jsonpe) { @@ -109,6 +101,29 @@ public void process() { } } + protected void processPackageEntry(JsonObject pkg) { + if (pkg.containsKey("name")) { + final String groupName = pkg.getString("name"); + if (groupName.indexOf('/') >= 0 && groupName.indexOf('/') <= groupName.length() - 1) { + if (pkg.containsKey("version")) { + final String group = groupName.substring(0, groupName.indexOf('/')); + final String project = groupName.substring(groupName.indexOf('/') + 1); + String version = pkg.getString("version"); + // Some version numbers begin with v - which doesn't end up matching CPE's + if (version.startsWith("v")) { + version = version.substring(1); + } + LOGGER.debug("Got package {}/{}/{}", group, project, version); + composerDependencies.add(new ComposerDependency(group, project, version)); + } else { + LOGGER.debug("Group/package {} does not have a version", groupName); + } + } else { + LOGGER.debug("Got a dependency with no name"); + } + } + } + /** * Gets the list of dependencies. * diff --git a/core/src/test/java/org/owasp/dependencycheck/data/composer/ComposerLockParserTest.java b/core/src/test/java/org/owasp/dependencycheck/data/composer/ComposerLockParserTest.java index 162258db6a5..4a2e9bfe724 100644 --- a/core/src/test/java/org/owasp/dependencycheck/data/composer/ComposerLockParserTest.java +++ b/core/src/test/java/org/owasp/dependencycheck/data/composer/ComposerLockParserTest.java @@ -43,30 +43,42 @@ public void setUp() throws Exception { @Test public void testValidComposerLock() { - ComposerLockParser clp = new ComposerLockParser(inputStream); + ComposerLockParser clp = new ComposerLockParser(inputStream, false); clp.process(); assertEquals(30, clp.getDependencies().size()); assertTrue(clp.getDependencies().contains(new ComposerDependency("symfony", "translation", "2.7.3"))); + assertTrue(clp.getDependencies().contains(new ComposerDependency("vlucas", "phpdotenv", "1.1.1"))); + } + + + @Test + public void testComposerLockSkipDev() { + ComposerLockParser clp = new ComposerLockParser(inputStream, true); + clp.process(); + assertEquals(29, clp.getDependencies().size()); + assertTrue(clp.getDependencies().contains(new ComposerDependency("symfony", "translation", "2.7.3"))); + //vlucas/phpdotenv is in packages-dev + assertFalse(clp.getDependencies().contains(new ComposerDependency("vlucas", "phpdotenv", "1.1.1"))); } @Test(expected = ComposerException.class) public void testNotJSON() throws Exception { String input = "NOT VALID JSON"; - ComposerLockParser clp = new ComposerLockParser(new ByteArrayInputStream(input.getBytes(Charset.defaultCharset()))); + ComposerLockParser clp = new ComposerLockParser(new ByteArrayInputStream(input.getBytes(Charset.defaultCharset())), false); clp.process(); } @Test(expected = ComposerException.class) public void testNotComposer() throws Exception { String input = "[\"ham\",\"eggs\"]"; - ComposerLockParser clp = new ComposerLockParser(new ByteArrayInputStream(input.getBytes(Charset.defaultCharset()))); + ComposerLockParser clp = new ComposerLockParser(new ByteArrayInputStream(input.getBytes(Charset.defaultCharset())), false); clp.process(); } @Test(expected = ComposerException.class) public void testNotPackagesArray() throws Exception { String input = "{\"packages\":\"eleventy\"}"; - ComposerLockParser clp = new ComposerLockParser(new ByteArrayInputStream(input.getBytes(Charset.defaultCharset()))); + ComposerLockParser clp = new ComposerLockParser(new ByteArrayInputStream(input.getBytes(Charset.defaultCharset())), false); clp.process(); } } diff --git a/core/src/test/resources/composer.lock b/core/src/test/resources/composer.lock index 29c47bb0377..fd43830bdd2 100644 --- a/core/src/test/resources/composer.lock +++ b/core/src/test/resources/composer.lock @@ -1687,7 +1687,9 @@ "dump" ], "time": "2015-07-28 15:18:12" - }, + } + ], + "packages-dev": [ { "name": "vlucas/phpdotenv", "version": "v1.1.1", @@ -1735,7 +1737,6 @@ "time": "2015-05-30 15:59:26" } ], - "packages-dev": [], "aliases": [], "minimum-stability": "stable", "stability-flags": [], diff --git a/maven/src/main/java/org/owasp/dependencycheck/maven/BaseDependencyCheckMojo.java b/maven/src/main/java/org/owasp/dependencycheck/maven/BaseDependencyCheckMojo.java index aca91bd1fca..159aa780c7e 100644 --- a/maven/src/main/java/org/owasp/dependencycheck/maven/BaseDependencyCheckMojo.java +++ b/maven/src/main/java/org/owasp/dependencycheck/maven/BaseDependencyCheckMojo.java @@ -490,6 +490,11 @@ public abstract class BaseDependencyCheckMojo extends AbstractMojo implements Ma */ @Parameter(property = "composerAnalyzerEnabled") private Boolean composerAnalyzerEnabled; + /** + * Sets whether or not the PHP Composer Lock File Analyzer will scan "packages-dev". + */ + @Parameter(property = "composerAnalyzerSkipDev") + private boolean composerAnalyzerSkipDev; /** * Whether or not the Perl CPAN File Analyzer is enabled. */ @@ -2282,6 +2287,7 @@ protected void populateSettings() { settings.setBooleanIfNotNull(Settings.KEYS.ANALYZER_PIPFILE_ENABLED, pipfileAnalyzerEnabled); settings.setBooleanIfNotNull(Settings.KEYS.ANALYZER_POETRY_ENABLED, poetryAnalyzerEnabled); settings.setBooleanIfNotNull(Settings.KEYS.ANALYZER_COMPOSER_LOCK_ENABLED, composerAnalyzerEnabled); + settings.setBooleanIfNotNull(Settings.KEYS.ANALYZER_COMPOSER_LOCK_SKIP_DEV, composerAnalyzerSkipDev); settings.setBooleanIfNotNull(Settings.KEYS.ANALYZER_CPANFILE_ENABLED, cpanfileAnalyzerEnabled); settings.setBooleanIfNotNull(Settings.KEYS.ANALYZER_NODE_PACKAGE_ENABLED, nodeAnalyzerEnabled); settings.setBooleanIfNotNull(Settings.KEYS.ANALYZER_NODE_AUDIT_ENABLED, nodeAuditAnalyzerEnabled); diff --git a/maven/src/site/markdown/configuration.md b/maven/src/site/markdown/configuration.md index e19a8c44053..fba95d8d463 100644 --- a/maven/src/site/markdown/configuration.md +++ b/maven/src/site/markdown/configuration.md @@ -85,8 +85,9 @@ cmakeAnalyzerEnabled | Sets whether the [experimental](../analyze autoconfAnalyzerEnabled | Sets whether the [experimental](../analyzers/index.html) autoconf Analyzer should be used. `enableExperimental` must be set to true. | true pipAnalyzerEnabled | Sets whether the [experimental](../analyzers/index.html) pip Analyzer should be used. `enableExperimental` must be set to true. | true pipfileAnalyzerEnabled | Sets whether the [experimental](../analyzers/index.html) Pipfile Analyzer should be used. `enableExperimental` must be set to true. | true -poetryAnalyzerEnabled | Sets whether the [experimental](../analyzers/index.html) Poetry Analyzer should be used. `enableExperimental` must be set to true. | true +poetryAnalyzerEnabled | Sets whether the [experimental](../analyzers/index.html) Poetry Analyzer should be used. `enableExperimental` must be set to true. | true composerAnalyzerEnabled | Sets whether the [experimental](../analyzers/index.html) PHP Composer Lock File Analyzer should be used. `enableExperimental` must be set to true. | true +composerAnalyzerSkipDev | Sets whether the [experimental](../analyzers/index.html) PHP Composer Lock File Analyzer should skip "packages-dev" | false cpanfileAnalyzerEnabled | Sets whether the [experimental](../analyzers/index.html) Perl CPAN File Analyzer should be used. `enableExperimental` must be set to true. | true yarnAuditAnalyzerEnabled | Sets whether the Yarn Audit Analyzer should be used. This analyzer requires yarn and an internet connection. Use `nodeAuditSkipDevDependencies` to skip dev dependencies. | true pnpmAuditAnalyzerEnabled | Sets whether the Pnpm Audit Analyzer should be used. This analyzer requires pnpm and an internet connection. Use `nodeAuditSkipDevDependencies` to skip dev dependencies. | true diff --git a/src/site/markdown/dependency-check-gradle/configuration.md b/src/site/markdown/dependency-check-gradle/configuration.md index bd7dbd92876..4ae38f60b09 100644 --- a/src/site/markdown/dependency-check-gradle/configuration.md +++ b/src/site/markdown/dependency-check-gradle/configuration.md @@ -123,6 +123,7 @@ analyzers | pathToDotnet | The path to dotnet core - needed on some analyzers | cmakeEnabled | Sets whether the [experimental](../analyzers/index.html) CMake Analyzer should be used. `experimentalEnabled` must be set to true. | true analyzers | autoconfEnabled | Sets whether the [experimental](../analyzers/index.html) autoconf Analyzer should be used. `experimentalEnabled` must be set to true. | true analyzers | composerEnabled | Sets whether the [experimental](../analyzers/index.html) PHP Composer Lock File Analyzer should be used. `experimentalEnabled` must be set to true. | true +analyzers | composerSkipDev | Sets whether the [experimental](../analyzers/index.html) PHP Composer Lock File Analyzer should skip "packages-dev". | false analyzers | cpanEnabled | Sets whether the [experimental](../analyzers/index.html) Perl CPAN File Analyzer should be used. `experimentalEnabled` must be set to true. | true analyzers | nodeEnabled | Sets whether the Node.js Analyzer should be used. | true analyzers | cocoapodsEnabled | Sets whether the [experimental](../analyzers/index.html) Cocoapods Analyzer should be used. `experimentalEnabled` must be set to true. | true diff --git a/utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java b/utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java index 7e262147448..ed5a48142da 100644 --- a/utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java +++ b/utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java @@ -449,6 +449,11 @@ public static final class KEYS { * enabled. */ public static final String ANALYZER_COMPOSER_LOCK_ENABLED = "analyzer.composer.lock.enabled"; + /** + * The properties key for whether the PHP composer lock file analyzer + * should skip dev packages. + */ + public static final String ANALYZER_COMPOSER_LOCK_SKIP_DEV = "analyzer.composer.lock.skipdev"; /** * The properties key for whether the Perl CPAN file file analyzer is * enabled.