Skip to content

Commit 8c8c19d

Browse files
ftiercelinjeremylong
ftiercelin
and
jeremylong
authored
feat: Add a flag to fail when one or more suppression rules are not used (#7244)
Co-authored-by: Jeremy Long <[email protected]>
1 parent ca4601b commit 8c8c19d

File tree

9 files changed

+322
-55
lines changed

9 files changed

+322
-55
lines changed

‎ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,11 @@ public class Check extends Update {
449449
*/
450450
private Reference refId = null;
451451

452+
/**
453+
* whether an unsused suppression rule should get force the build to fail
454+
*/
455+
private boolean failBuildOnUnusedSuppressionRule = false;
456+
452457
/**
453458
* Returns whether the version check is enabled.
454459
*
@@ -2092,6 +2097,24 @@ public String getArtifactoryAnalyzerBearerToken() {
20922097
public void setArtifactoryAnalyzerBearerToken(String artifactoryAnalyzerBearerToken) {
20932098
this.artifactoryAnalyzerBearerToken = artifactoryAnalyzerBearerToken;
20942099
}
2100+
2101+
/**
2102+
* Returns the value of failBuildOnUnusedSuppressionRule.
2103+
* @return whether an unsused suppression rule should get force the build to fail
2104+
*/
2105+
public boolean getFailBuildOnUnusedSuppressionRule() {
2106+
return failBuildOnUnusedSuppressionRule;
2107+
}
2108+
2109+
/**
2110+
* Set the value of failBuildOnUnusedSuppressionRule.
2111+
*
2112+
* @param failBuildOnUnusedSuppressionRule new value of
2113+
* failBuildOnUnusedSuppressionRule
2114+
*/
2115+
public void setFailBuildOnUnusedSuppressionRule(boolean failBuildOnUnusedSuppressionRule) {
2116+
this.failBuildOnUnusedSuppressionRule = failBuildOnUnusedSuppressionRule;
2117+
}
20952118

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

22852309
/**

‎ant/src/site/markdown/configuration.md‎

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -30,27 +30,28 @@ Configuration: dependency-check Task
3030
--------------------
3131
The following properties can be set on the dependency-check task.
3232

33-
Property | Description | Default Value
34-
----------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-----------------
35-
autoUpdate | Sets whether auto-updating of the NVD CVE/CPE data is enabled. It is not recommended that this be turned to false. | true
36-
failOnError | Whether the build should fail if there is an error executing the dependency-check analysis | true
37-
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
38-
junitFailOnCVSS | If using the JUNIT report format the junitFailOnCVSS sets the CVSS score threshold that is considered a failure. | 0
39-
prettyPrint | Whether the XML and JSON formatted reports should be pretty printed. | false
40-
projectName | The name of the project being scanned. | Dependency-Check
41-
reportFormat | The report format to be generated (HTML, XML, CSV, JSON, JUNIT, SARIF, JENKINS, GITLAB, ALL). | HTML
42-
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'
43-
hintsFile | The file path to the XML hints file \- used to resolve [false negatives](../general/hints.html) | &nbsp;
44-
proxyServer | The Proxy Server; see the [proxy configuration](../data/proxy.html) page for more information. | &nbsp;
45-
proxyPort | The Proxy Port. | &nbsp;
46-
proxyUsername | Defines the proxy user name. | &nbsp;
47-
proxyPassword | Defines the proxy password. | &nbsp;
48-
nonProxyHosts | Defines the hosts that will not be proxied. | &nbsp;
49-
connectionTimeout | The URL Connection Timeout. | &nbsp;
50-
enableExperimental | Enable the [experimental analyzers](../analyzers/index.html). If not enabled the experimental analyzers (see below) will not be loaded or used. | false
51-
enableRetired | Enable the [retired analyzers](../analyzers/index.html). If not enabled the retired analyzers (see below) will not be loaded or used. | false
52-
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;
53-
junitFailOnCVSS | If using the JUNIT report format the junitFailOnCVSS sets the CVSS score threshold that is considered a failure. | 0
33+
Property | Description | Default Value
34+
---------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-----------------
35+
autoUpdate | Sets whether auto-updating of the NVD CVE/CPE data is enabled. It is not recommended that this be turned to false. | true
36+
failOnError | Whether the build should fail if there is an error executing the dependency-check analysis | true
37+
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
38+
junitFailOnCVSS | If using the JUNIT report format the junitFailOnCVSS sets the CVSS score threshold that is considered a failure. | 0
39+
prettyPrint | Whether the XML and JSON formatted reports should be pretty printed. | false
40+
projectName | The name of the project being scanned. | Dependency-Check
41+
reportFormat | The report format to be generated (HTML, XML, CSV, JSON, JUNIT, SARIF, JENKINS, GITLAB, ALL). | HTML
42+
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'
43+
hintsFile | The file path to the XML hints file \- used to resolve [false negatives](../general/hints.html) | &nbsp;
44+
proxyServer | The Proxy Server; see the [proxy configuration](../data/proxy.html) page for more information. | &nbsp;
45+
proxyPort | The Proxy Port. | &nbsp;
46+
proxyUsername | Defines the proxy user name. | &nbsp;
47+
proxyPassword | Defines the proxy password. | &nbsp;
48+
nonProxyHosts | Defines the hosts that will not be proxied. | &nbsp;
49+
connectionTimeout | The URL Connection Timeout. | &nbsp;
50+
enableExperimental | Enable the [experimental analyzers](../analyzers/index.html). If not enabled the experimental analyzers (see below) will not be loaded or used. | false
51+
enableRetired | Enable the [retired analyzers](../analyzers/index.html). If not enabled the retired analyzers (see below) will not be loaded or used. | false
52+
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;
53+
junitFailOnCVSS | If using the JUNIT report format the junitFailOnCVSS sets the CVSS score threshold that is considered a failure. | 0
54+
failBuildOnUnusedSuppressionRule | Specifies that if any unused suppression rule is found, the build will fail. | false
5455

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

‎core/src/main/java/org/owasp/dependencycheck/agent/DependencyCheckScanAgent.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,10 @@ public class DependencyCheckScanAgent {
177177
* Whether the Maven Central analyzer is enabled.
178178
*/
179179
private boolean centralAnalyzerEnabled = true;
180+
/**
181+
* Whether the build should fail if there are unused suppression rules.
182+
*/
183+
private boolean failOnUnusedSuppressionRule = false;
180184
/**
181185
* The URL of Maven Central.
182186
*/
@@ -619,6 +623,24 @@ public String getCpeStartsWithFilter() {
619623
return cpeStartsWithFilter;
620624
}
621625

626+
/**
627+
* Get the value of failOnUnusedSuppressionRule.
628+
*
629+
* @return the value of failOnUnusedSuppressionRule
630+
*/
631+
public boolean isFailOnUnusedSuppressionRule() {
632+
return failOnUnusedSuppressionRule;
633+
}
634+
635+
/**
636+
* Set the value of failOnUnusedSuppressionRule.
637+
*
638+
* @param failOnUnusedSuppressionRule new value of failOnUnusedSuppressionRule
639+
*/
640+
public void setFailOnUnusedSuppressionRule(boolean failOnUnusedSuppressionRule) {
641+
this.failOnUnusedSuppressionRule = failOnUnusedSuppressionRule;
642+
}
643+
622644
/**
623645
* Get the value of centralAnalyzerEnabled.
624646
*
@@ -952,6 +974,7 @@ private void populateSettings() {
952974
settings.setStringIfNotEmpty(Settings.KEYS.ADDITIONAL_ZIP_EXTENSIONS, zipExtensions);
953975
settings.setStringIfNotEmpty(Settings.KEYS.NVD_API_KEY, nvdApiKey);
954976
settings.setStringIfNotEmpty(Settings.KEYS.ANALYZER_ASSEMBLY_DOTNET_PATH, pathToCore);
977+
settings.setBoolean(Settings.KEYS.FAIL_ON_UNUSED_SUPPRESSION_RULE, failOnUnusedSuppressionRule);
955978
}
956979

957980
/**

‎core/src/main/java/org/owasp/dependencycheck/analyzer/UnusedSuppressionRuleAnalyzer.java‎

Lines changed: 55 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@
3333
* @author Jeremy Long
3434
*/
3535
public class UnusedSuppressionRuleAnalyzer extends AbstractAnalyzer {
36-
36+
protected static final String EXCEPTION_MSG = "There are %d unused suppression rule(s): check logs.";
37+
3738
/**
3839
* The Logger for use throughout the class.
3940
*/
@@ -43,27 +44,54 @@ public class UnusedSuppressionRuleAnalyzer extends AbstractAnalyzer {
4344
* been reported.
4445
*/
4546
private boolean reported = false;
47+
/**
48+
* A flag indicating whether build should fail on unused suppression rule
49+
*/
50+
private boolean shouldFailForUnusedSuppressionRule = false;
51+
/**
52+
* unused suppression rule count
53+
*/
54+
private int unusedSuppressionRuleCount = 0;
55+
56+
@Override
57+
public synchronized void initialize(Settings settings) {
58+
super.initialize(settings);
59+
if (settings.getBoolean(Settings.KEYS.FAIL_ON_UNUSED_SUPPRESSION_RULE, false)) {
60+
this.shouldFailForUnusedSuppressionRule = true;
61+
}
62+
}
4663

4764
@Override
4865
protected void analyzeDependency(Dependency dependency, Engine engine) throws AnalysisException {
4966
if (!reported) {
50-
logUnusedRules(engine);
51-
reported = true;
67+
checkUnusedRules(engine);
68+
reported = true;
69+
if(unusedSuppressionRuleCount > 0 && failsForUnusedSuppressionRule()) {
70+
final String message = String.format(EXCEPTION_MSG, unusedSuppressionRuleCount);
71+
LOGGER.error(message);
72+
throw new AnalysisException(message);
73+
}
5274
}
5375
}
5476

5577
/**
56-
* Logs unused suppression RULES.
78+
* check unused suppression RULES.
5779
*
5880
* @param engine a reference to the ODC engine
5981
*/
60-
private void logUnusedRules(Engine engine) {
82+
protected void checkUnusedRules(Engine engine) {
6183
if (engine.hasObject(SUPPRESSION_OBJECT_KEY)) {
6284
@SuppressWarnings("unchecked")
6385
final List<SuppressionRule> rules = (List<SuppressionRule>) engine.getObject(SUPPRESSION_OBJECT_KEY);
6486
rules.forEach((rule) -> {
6587
if (!rule.isMatched() && !rule.isBase()) {
66-
LOGGER.info("Suppression Rule had zero matches: {}", rule.toString());
88+
final String message = String.format("Suppression Rule had zero matches: %s", rule);
89+
if(failsForUnusedSuppressionRule()) {
90+
LOGGER.error(message);
91+
} else {
92+
LOGGER.info(message);
93+
}
94+
increaseUnusedSuppressionRuleCount();
6795
}
6896
});
6997
}
@@ -89,4 +117,25 @@ public AnalysisPhase getAnalysisPhase() {
89117
public boolean supportsParallelProcessing() {
90118
return false;
91119
}
120+
121+
/**
122+
* increases the count of unused suppression rules
123+
*/
124+
public void increaseUnusedSuppressionRuleCount() {
125+
unusedSuppressionRuleCount++;
126+
}
127+
128+
/**
129+
* @return the count of unused suppression rules
130+
*/
131+
public int getUnusedSuppressionRuleCount() {
132+
return unusedSuppressionRuleCount;
133+
}
134+
135+
/**
136+
* @return whether the analyzer will fail for a unused suppression rule
137+
*/
138+
public boolean failsForUnusedSuppressionRule() {
139+
return shouldFailForUnusedSuppressionRule;
140+
}
92141
}

‎core/src/main/resources/dependencycheck.properties‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ database.batchinsert.maxsize=1000
164164
analyzer.artifactory.enabled=false
165165
analyzer.libman.enabled=true
166166
odc.reports.pretty.print=false
167+
analyzer.suppression.unused.fail=false
167168

168169
hosted.suppressions.enabled=true
169170
hosted.suppressions.url=https://jeremylong.github.io/DependencyCheck/suppressions/publishedSuppressions.xml

0 commit comments

Comments
 (0)