Skip to content

Commit

Permalink
Significant progress towards generalizing scoring to support other test
Browse files Browse the repository at this point in the history 
suites. General infrastructure in place and about 10 tool parsers converted
to support it. Also added some additional CWE mappings to a few tools.
  • Loading branch information
Dave Wichers committed Aug 16, 2024
1 parent 9050fd2 commit 1f175e7
Show file tree
Hide file tree
Showing 125 changed files with 1,311 additions and 798 deletions.
13 changes: 7 additions & 6 deletions plugin/src/main/java/org/owasp/benchmarkutils/score/BenchmarkScore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ public void execute() {

/**
* This is the original main() method used to invoke the scorecard generator. e.g., mvn validate
* -Pscorecard -Dexec.args="-cf ../julietjs/config/julietscoringconfig.yaml"
* -Pscorecard -Dexec.args="-cf ../TESTSUITENAME/config/testsuitescoringconfig.yaml"
*
* @param args - The command line arguments.
*/
Expand Down Expand Up @@ -576,15 +576,15 @@ private static void printExtraCWE(
TestSuiteResults expectedResults, TestSuiteResults actualResults) {
Set<Integer> expectedCWE = new HashSet<Integer>();
for (String testcase : expectedResults.keySet()) {
List<TestCaseResult> list = expectedResults.get(testcase);
List<TestCaseResult> list = expectedResults.getTestCaseResults(testcase);
for (TestCaseResult t : list) {
expectedCWE.add(t.getCWE());
}
}

Set<Integer> actualCWE = new HashSet<Integer>();
for (String testcase : actualResults.keySet()) {
List<TestCaseResult> list = actualResults.get(testcase);
List<TestCaseResult> list = actualResults.getTestCaseResults(testcase);
if (list != null) {
for (TestCaseResult t : list) {
actualCWE.add(t.getCWE());
Expand Down Expand Up @@ -683,7 +683,7 @@ private static Map<String, TP_FN_TN_FP_Counts> calculateScores(TestSuiteResults
Map<String, TP_FN_TN_FP_Counts> map = new TreeMap<String, TP_FN_TN_FP_Counts>();

for (String testcase : actualResults.keySet()) {
TestCaseResult tcr = actualResults.get(testcase).get(0); // only one
TestCaseResult tcr = actualResults.getTestCaseResults(testcase).get(0); // only one
String cat = Categories.getById(tcr.getCategory()).getName();

TP_FN_TN_FP_Counts c = map.get(cat);
Expand Down Expand Up @@ -764,9 +764,10 @@ private static TestSuiteResults analyze(

boolean pass = false;
for (String testcase : expected.keySet()) {
TestCaseResult exp = expected.get(testcase).get(0); // always only one!
TestCaseResult exp = expected.getTestCaseResults(testcase).get(0); // always only one!
List<TestCaseResult> act =
rawToolResults.get(testcase); // could be lots of results for this test
rawToolResults.getTestCaseResults(
testcase); // could be lots of results for this test

pass = compare(exp, act, rawToolResults.getToolName());

Expand Down
133 changes: 116 additions & 17 deletions plugin/src/main/java/org/owasp/benchmarkutils/score/TestCaseResult.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

import org.owasp.benchmarkutils.helpers.Categories;
import org.owasp.benchmarkutils.helpers.Category;
import org.owasp.benchmarkutils.score.parsers.Reader;
import org.owasp.benchmarkutils.score.service.ExpectedResultsProvider;
import org.owasp.benchmarkutils.tools.AbstractTestCaseRequest;

/* This class represents a single test case result. It documents the expected result (real),
Expand All @@ -28,7 +30,9 @@
public class TestCaseResult {

private String testCaseName = ""; // The name of the test case (E.g., BenchmarkTest00001)
private int number = 0;
// testID is the unique ID for this test case. For Benchmark Style, its a number (e.g., 1), for
// nonBenchmark Style its the name of the entire test case (e.g., FooBar_TryThis02).
private String testID = "0";
private boolean truePositive = false; // Is this test case a true or false positive?
private boolean result = false; // Did a tool properly detect this as a true or false positive?
private int CWE = 0;
Expand Down Expand Up @@ -58,10 +62,8 @@ public TestCaseResult() {
*/
public TestCaseResult(AbstractTestCaseRequest request) {
this.testCaseName = request.getName();
this.number = request.getNumber();
this.truePositive = request.isVulnerability();
this.CWE = request.getCategory().getCWE();
// this.category = request.getCategory().getName();
this.category = Categories.getByCWE(this.CWE).getName();

// fill in optional attributes since we have this data available
Expand All @@ -82,43 +84,122 @@ public void setTestCaseName(String name) {
* The name of the test case. E.g., BenchmarkTest00001
*/
public String getTestCaseName() {
return testCaseName;
return this.testCaseName;
}

public int getConfidence() {
return confidence;
return this.confidence;
}

public void setConfidence(int confidence) {
this.confidence = confidence;
}

public int getNumber() {
return number;
public String getTestID() {
return this.testID;
}

public void setNumber(int number) {
this.number = number;
/**
* Sets the unique identifier for this test case. For Benchmark style scoring, its the test ID
* number, converted to a String.
*
* <p>The use of this method should be converted to use setActualResultTestID() for tool
* parsers.
*
* @param id The unique test case number for this Benchmark style test case.
*/
@Deprecated
public void setTestID(int id) {
this.testID = String.valueOf(id);
}

/**
* Sets the unique identifier for this test case. For Benchmark style, it parses out the test
* case number and uses that as the test ID. For non-Benchmark style scoring, it used the name
* of the tool result file to try to find the matching expected result test case name. In this
* case, if the reported filename from the tool starts with the name of a test case, then the
* test case name, is used as the testID. That way if there are multi-file test cases that all
* start with the same name, they will all match up against the expected result name with that
* name.
*
* @param id The test case file name, without path information.
*/
public void setActualResultTestID(String testCaseFileName) {
if (ExpectedResultsProvider.isBenchmarkStyleScoring()) {
// Sets the test ID to the test case # or -1 if not a match
this.testID =
String.valueOf(Reader.getBenchmarkStyleTestCaseNumber(testCaseFileName.trim()));
} else {
if (testCaseFileName.contains("/") || testCaseFileName.contains("\\")) {
new IllegalArgumentException(
"FATAL ERROR: testCaseFileName value: "
+ testCaseFileName
+ " passed to setActualResultTestID() can't have any path information")
.printStackTrace();
System.exit(-1);
}
// For actual results, we look for a matching test case name, and set that as the testID
String matchingID =
ExpectedResultsProvider.getExpectedResults()
.getMatchingTestCaseName(testCaseFileName);
// TODO: Maybe null is OK, and we should simply set the test ID to -1, like we do for
// Benchmark
if (matchingID == null) {
new IllegalArgumentException(
"FATAL ERROR: testCaseFileName value: "
+ testCaseFileName
+ " passed to setActualResultTestID() doesn't match any expected results test case name.")
.printStackTrace();
System.exit(-1);
}
this.testID = matchingID;
}
}

/**
* Sets the unique identifier for this test case. For non-Benchmark style scoring, its the name
* of the test case. For Benchmark style, it parses out the test case number and uses that as
* the test ID.
*
* @param id The test case file name, without path information.
*/
public void setExpectedResultTestID(String testCaseFileName) {
if (ExpectedResultsProvider.isBenchmarkStyleScoring()) {
// Sets the test ID to the test case # or -1 if not a match
this.testID =
String.valueOf(Reader.getBenchmarkStyleTestCaseNumber(testCaseFileName.trim()));
} else {
if (testCaseFileName.contains("/") || testCaseFileName.contains("\\")) {
new IllegalArgumentException(
"FATAL ERROR: testCaseFileName value: "
+ testCaseFileName
+ " passed to setExpectedResultTestID() can't have any path information")
.printStackTrace();
System.exit(-1);
}
// For expected results, we don't change the test case file name
this.testID = testCaseFileName.trim();
}
}

public boolean isTruePositive() {
return truePositive;
return this.truePositive;
}

public void setTruePositive(boolean truePositive) {
this.truePositive = truePositive;
}

public boolean isPassed() {
return result;
return this.result;
}

public void setPassed(boolean result) {
this.result = result;
}

public int getCWE() {
return CWE;
return this.CWE;
}

public void setCWE(int cwe) {
Expand All @@ -127,7 +208,25 @@ public void setCWE(int cwe) {
if (category != null) {
this.category = category.getId();
} else {
this.category = this.UNMAPPED_CATEGORY;
this.category = TestCaseResult.UNMAPPED_CATEGORY;
}
}

/**
* This method is used to abstract away how filenames are matched against a test case, that way
* it can be enhanced to support different test suite formats, and the logic on how to do this
* isn't implemented in every individual parser. This method expects that
* ExpectedResultsProvider.getExpectedResults().isTestCaseFile(filename) was called first to
* verify this file is a test case in the test suite being scored. It sets the CWE number and
* the test case name in this TestCaseResult if successful. It halts with an error if the
* supplied filename is not a valid test case.
*
* @param cwe The CWE # reported by this tool.
* @param filename The filename that might be a test case.
*/
public void setCWEAndTestCaseID(int cwe, String filename) {
if (ExpectedResultsProvider.getExpectedResults().isTestCaseFile(filename)) {
// TODO
}
}

Expand All @@ -137,7 +236,7 @@ public void setCWE(int cwe) {
* @return The descriptive name of this CWE, per categories.xml
*/
public String getCategory() {
return category;
return this.category;
}

/*
Expand All @@ -155,7 +254,7 @@ public void setCategory(String category) {
}
*/
public String getEvidence() {
return evidence;
return this.evidence;
}

public void setEvidence(String evidence) {
Expand Down Expand Up @@ -188,8 +287,8 @@ public void setSink(String sink) {

@Override
public String toString() {
return "Testcase #: "
+ getNumber()
return "Testcase ID: "
+ getTestID()
+ ", Category: "
+ getCategory()
+ ", isVulnerable: "
Expand Down
Loading

0 comments on commit 1f175e7

Please sign in to comment.