Skip to content

Commit

Permalink
feat: Add a flag to fail when one or more suppression rules are not u…
Browse files Browse the repository at this point in the history 
…sed (#7244)

Co-authored-by: Jeremy Long <[email protected]>
  • Loading branch information
@ftiercelin @jeremylong
ftiercelin and jeremylong authored Dec 20, 2024
1 parent ca4601b commit 8c8c19d
Show file tree
Hide file tree
Showing 9 changed files with 322 additions and 55 deletions.
24 changes: 24 additions & 0 deletions ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,11 @@ public class Check extends Update {
*/
private Reference refId = null;

/**
* whether an unsused suppression rule should get force the build to fail
*/
private boolean failBuildOnUnusedSuppressionRule = false;

/**
* Returns whether the version check is enabled.
*
Expand Down Expand Up @@ -2092,6 +2097,24 @@ public String getArtifactoryAnalyzerBearerToken() {
public void setArtifactoryAnalyzerBearerToken(String artifactoryAnalyzerBearerToken) {
this.artifactoryAnalyzerBearerToken = artifactoryAnalyzerBearerToken;
}

/**
* Returns the value of failBuildOnUnusedSuppressionRule.
* @return whether an unsused suppression rule should get force the build to fail
*/
public boolean getFailBuildOnUnusedSuppressionRule() {
return failBuildOnUnusedSuppressionRule;
}

/**
* Set the value of failBuildOnUnusedSuppressionRule.
*
* @param failBuildOnUnusedSuppressionRule new value of
* failBuildOnUnusedSuppressionRule
*/
public void setFailBuildOnUnusedSuppressionRule(boolean failBuildOnUnusedSuppressionRule) {
this.failBuildOnUnusedSuppressionRule = failBuildOnUnusedSuppressionRule;
}

//see note on `dealWithReferences()` for information on this suppression
@SuppressWarnings("squid:RedundantThrowsDeclarationCheck")
Expand Down Expand Up @@ -2280,6 +2303,7 @@ protected void populateSettings() throws BuildException {
getSettings().setBooleanIfNotNull(Settings.KEYS.ANALYZER_OSSINDEX_USE_CACHE, ossindexAnalyzerUseCache);
getSettings().setBooleanIfNotNull(Settings.KEYS.ANALYZER_OSSINDEX_WARN_ONLY_ON_REMOTE_ERRORS, ossIndexAnalyzerWarnOnlyOnRemoteErrors);
getSettings().setFloat(Settings.KEYS.JUNIT_FAIL_ON_CVSS, junitFailOnCVSS);
getSettings().setBooleanIfNotNull(Settings.KEYS.FAIL_ON_UNUSED_SUPPRESSION_RULE, failBuildOnUnusedSuppressionRule);
}

/**
Expand Down
43 changes: 22 additions & 21 deletions ant/src/site/markdown/configuration.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,27 +30,28 @@ Configuration: dependency-check Task
--------------------
The following properties can be set on the dependency-check task.

Property | Description | Default Value
----------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-----------------
autoUpdate | Sets whether auto-updating of the NVD CVE/CPE data is enabled. It is not recommended that this be turned to false. | true
failOnError | Whether the build should fail if there is an error executing the dependency-check analysis | true
failBuildOnCVSS | Specifies if the build should be failed if a CVSS score equal to or above a specified level is identified. The default is 11 which means since the CVSS scores are 0-10, by default the build will never fail. More information on CVSS scores can be found at the [NVD](https://nvd.nist.gov/vuln-metrics/cvss)| 11
junitFailOnCVSS | If using the JUNIT report format the junitFailOnCVSS sets the CVSS score threshold that is considered a failure. | 0
prettyPrint | Whether the XML and JSON formatted reports should be pretty printed. | false
projectName | The name of the project being scanned. | Dependency-Check
reportFormat | The report format to be generated (HTML, XML, CSV, JSON, JUNIT, SARIF, JENKINS, GITLAB, ALL). | 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'
hintsFile | The file path to the XML hints file \- used to resolve [false negatives](../general/hints.html) | &nbsp;
proxyServer | The Proxy Server; see the [proxy configuration](../data/proxy.html) page for more information. | &nbsp;
proxyPort | The Proxy Port. | &nbsp;
proxyUsername | Defines the proxy user name. | &nbsp;
proxyPassword | Defines the proxy password. | &nbsp;
nonProxyHosts | Defines the hosts that will not be proxied. | &nbsp;
connectionTimeout | The URL Connection Timeout. | &nbsp;
enableExperimental | Enable the [experimental analyzers](../analyzers/index.html). If not enabled the experimental analyzers (see below) will not be loaded or used. | false
enableRetired | Enable the [retired analyzers](../analyzers/index.html). If not enabled the retired analyzers (see below) will not be loaded or used. | false
suppressionFile | The file path to the XML suppression file \- used to suppress [false positives](../general/suppression.html). The parameter value can be a local file path, a URL to a suppression file, or even a reference to a file on the class path (see https://github.com/jeremylong/DependencyCheck/issues/1878#issuecomment-487533799) | &nbsp;
junitFailOnCVSS | If using the JUNIT report format the junitFailOnCVSS sets the CVSS score threshold that is considered a failure. | 0
Property | Description | Default Value
---------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-----------------
autoUpdate | Sets whether auto-updating of the NVD CVE/CPE data is enabled. It is not recommended that this be turned to false. | true
failOnError | Whether the build should fail if there is an error executing the dependency-check analysis | true
failBuildOnCVSS | Specifies if the build should be failed if a CVSS score equal to or above a specified level is identified. The default is 11 which means since the CVSS scores are 0-10, by default the build will never fail. More information on CVSS scores can be found at the [NVD](https://nvd.nist.gov/vuln-metrics/cvss)| 11
junitFailOnCVSS | If using the JUNIT report format the junitFailOnCVSS sets the CVSS score threshold that is considered a failure. | 0
prettyPrint | Whether the XML and JSON formatted reports should be pretty printed. | false
projectName | The name of the project being scanned. | Dependency-Check
reportFormat | The report format to be generated (HTML, XML, CSV, JSON, JUNIT, SARIF, JENKINS, GITLAB, ALL). | 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'
hintsFile | The file path to the XML hints file \- used to resolve [false negatives](../general/hints.html) | &nbsp;
proxyServer | The Proxy Server; see the [proxy configuration](../data/proxy.html) page for more information. | &nbsp;
proxyPort | The Proxy Port. | &nbsp;
proxyUsername | Defines the proxy user name. | &nbsp;
proxyPassword | Defines the proxy password. | &nbsp;
nonProxyHosts | Defines the hosts that will not be proxied. | &nbsp;
connectionTimeout | The URL Connection Timeout. | &nbsp;
enableExperimental | Enable the [experimental analyzers](../analyzers/index.html). If not enabled the experimental analyzers (see below) will not be loaded or used. | false
enableRetired | Enable the [retired analyzers](../analyzers/index.html). If not enabled the retired analyzers (see below) will not be loaded or used. | false
suppressionFile | The file path to the XML suppression file \- used to suppress [false positives](../general/suppression.html). The parameter value can be a local file path, a URL to a suppression file, or even a reference to a file on the class path (see https://github.com/jeremylong/DependencyCheck/issues/1878#issuecomment-487533799) | &nbsp;
junitFailOnCVSS | If using the JUNIT report format the junitFailOnCVSS sets the CVSS score threshold that is considered a failure. | 0
failBuildOnUnusedSuppressionRule | Specifies that if any unused suppression rule is found, the build will fail. | false

The following nested elements can be set on the dependency-check task.

Expand Down
23 changes: 23 additions & 0 deletions core/src/main/java/org/owasp/dependencycheck/agent/DependencyCheckScanAgent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,10 @@ public class DependencyCheckScanAgent {
* Whether the Maven Central analyzer is enabled.
*/
private boolean centralAnalyzerEnabled = true;
/**
* Whether the build should fail if there are unused suppression rules.
*/
private boolean failOnUnusedSuppressionRule = false;
/**
* The URL of Maven Central.
*/
Expand Down Expand Up @@ -619,6 +623,24 @@ public String getCpeStartsWithFilter() {
return cpeStartsWithFilter;
}

/**
* Get the value of failOnUnusedSuppressionRule.
*
* @return the value of failOnUnusedSuppressionRule
*/
public boolean isFailOnUnusedSuppressionRule() {
return failOnUnusedSuppressionRule;
}

/**
* Set the value of failOnUnusedSuppressionRule.
*
* @param failOnUnusedSuppressionRule new value of failOnUnusedSuppressionRule
*/
public void setFailOnUnusedSuppressionRule(boolean failOnUnusedSuppressionRule) {
this.failOnUnusedSuppressionRule = failOnUnusedSuppressionRule;
}

/**
* Get the value of centralAnalyzerEnabled.
*
Expand Down Expand Up @@ -952,6 +974,7 @@ private void populateSettings() {
settings.setStringIfNotEmpty(Settings.KEYS.ADDITIONAL_ZIP_EXTENSIONS, zipExtensions);
settings.setStringIfNotEmpty(Settings.KEYS.NVD_API_KEY, nvdApiKey);
settings.setStringIfNotEmpty(Settings.KEYS.ANALYZER_ASSEMBLY_DOTNET_PATH, pathToCore);
settings.setBoolean(Settings.KEYS.FAIL_ON_UNUSED_SUPPRESSION_RULE, failOnUnusedSuppressionRule);
}

/**
Expand Down
61 changes: 55 additions & 6 deletions core/src/main/java/org/owasp/dependencycheck/analyzer/UnusedSuppressionRuleAnalyzer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@
* @author Jeremy Long
*/
public class UnusedSuppressionRuleAnalyzer extends AbstractAnalyzer {

protected static final String EXCEPTION_MSG = "There are %d unused suppression rule(s): check logs.";

/**
* The Logger for use throughout the class.
*/
Expand All @@ -43,27 +44,54 @@ public class UnusedSuppressionRuleAnalyzer extends AbstractAnalyzer {
* been reported.
*/
private boolean reported = false;
/**
* A flag indicating whether build should fail on unused suppression rule
*/
private boolean shouldFailForUnusedSuppressionRule = false;
/**
* unused suppression rule count
*/
private int unusedSuppressionRuleCount = 0;

@Override
public synchronized void initialize(Settings settings) {
super.initialize(settings);
if (settings.getBoolean(Settings.KEYS.FAIL_ON_UNUSED_SUPPRESSION_RULE, false)) {
this.shouldFailForUnusedSuppressionRule = true;
}
}

@Override
protected void analyzeDependency(Dependency dependency, Engine engine) throws AnalysisException {
if (!reported) {
logUnusedRules(engine);
reported = true;
checkUnusedRules(engine);
reported = true;
if(unusedSuppressionRuleCount > 0 && failsForUnusedSuppressionRule()) {
final String message = String.format(EXCEPTION_MSG, unusedSuppressionRuleCount);
LOGGER.error(message);
throw new AnalysisException(message);
}
}
}

/**
* Logs unused suppression RULES.
* check unused suppression RULES.
*
* @param engine a reference to the ODC engine
*/
private void logUnusedRules(Engine engine) {
protected void checkUnusedRules(Engine engine) {
if (engine.hasObject(SUPPRESSION_OBJECT_KEY)) {
@SuppressWarnings("unchecked")
final List<SuppressionRule> rules = (List<SuppressionRule>) engine.getObject(SUPPRESSION_OBJECT_KEY);
rules.forEach((rule) -> {
if (!rule.isMatched() && !rule.isBase()) {
LOGGER.info("Suppression Rule had zero matches: {}", rule.toString());
final String message = String.format("Suppression Rule had zero matches: %s", rule);
if(failsForUnusedSuppressionRule()) {
LOGGER.error(message);
} else {
LOGGER.info(message);
}
increaseUnusedSuppressionRuleCount();
}
});
}
Expand All @@ -89,4 +117,25 @@ public AnalysisPhase getAnalysisPhase() {
public boolean supportsParallelProcessing() {
return false;
}

/**
* increases the count of unused suppression rules
*/
public void increaseUnusedSuppressionRuleCount() {
unusedSuppressionRuleCount++;
}

/**
* @return the count of unused suppression rules
*/
public int getUnusedSuppressionRuleCount() {
return unusedSuppressionRuleCount;
}

/**
* @return whether the analyzer will fail for a unused suppression rule
*/
public boolean failsForUnusedSuppressionRule() {
return shouldFailForUnusedSuppressionRule;
}
}
1 change: 1 addition & 0 deletions core/src/main/resources/dependencycheck.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ database.batchinsert.maxsize=1000
analyzer.artifactory.enabled=false
analyzer.libman.enabled=true
odc.reports.pretty.print=false
analyzer.suppression.unused.fail=false

hosted.suppressions.enabled=true
hosted.suppressions.url=https://jeremylong.github.io/DependencyCheck/suppressions/publishedSuppressions.xml
Expand Down
Loading

0 comments on commit 8c8c19d

Please sign in to comment.