Skip to content

Commit

Permalink
Additional work in progress. Changed testcase expected/actual results
Browse files Browse the repository at this point in the history
map to use a String instead of int. For normal Benchmark scoring, the
results key is the testcase number, now as a string representation of
the test case number. For custom scoring, the results key is the name of
the entire test case file. Still have to implement changes to each
tool parser to be able to parse out either the test case number, or
provide the entire filename, depending on the scoring approach.
  • Loading branch information
Dave Wichers committed Aug 7, 2024
1 parent 1b27b66 commit d1aa364
Show file tree
Hide file tree
Showing 53 changed files with 180 additions and 163 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@ public class Category implements Comparable<Category> {
*/
public Category(String id, String name, int cwe, boolean isInjection, String shortname) {
this.id = id;
if (name.contains("/") || name.contains("\\")) {
System.out.println(
"FATAL ERROR: CWE name from provided categories.xml file: '"
+ name
+ "' contains a path character, which breaks scorecard generation.");
System.exit(-1);
}
this.name = name;
this.CWE = cwe;
this.isInjection = isInjection;
Expand Down
2 changes: 1 addition & 1 deletion library/src/main/resources/categories.xml
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@
</category>
<category>
<id>reflecti</id>
<name>Reflection</name>
<name>Unsafe Reflection</name>
<cwe>470</cwe>
<isInjection>true</isInjection>
<shortname>REFL</shortname>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -575,16 +575,16 @@ private static void process(
private static void printExtraCWE(
TestSuiteResults expectedResults, TestSuiteResults actualResults) {
Set<Integer> expectedCWE = new HashSet<Integer>();
for (int i : expectedResults.keySet()) {
List<TestCaseResult> list = expectedResults.get(i);
for (String testcase : expectedResults.keySet()) {
List<TestCaseResult> list = expectedResults.get(testcase);
for (TestCaseResult t : list) {
expectedCWE.add(t.getCWE());
}
}

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

for (Integer tn : actualResults.keySet()) {
TestCaseResult tcr = actualResults.get(tn).get(0); // only one
for (String testcase : actualResults.keySet()) {
TestCaseResult tcr = actualResults.get(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 @@ -763,17 +763,16 @@ private static TestSuiteResults analyze(
}

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

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

// helpful in debugging
// System.out.println( tc + ", " + exp.getCategory() + ", " + exp.isTruePositive() + ",
// " +
// exp.getCWE() + ", " + pass + "\n");
// System.out.println( testcase + ", " + exp.getCategory() + ", " + exp.isTruePositive()
// + "," + exp.getCWE() + ", " + pass + "\n");

// fill the result into the "expected" results in case we need it later
exp.setPassed(pass);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public void setTestCaseName(String name) {
/*
* The name of the test case. E.g., BenchmarkTest00001
*/
public String getName() {
public String getTestCaseName() {
return testCaseName;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,15 @@ public static enum ToolType {
// The name and version of the test suite these test results are for
private String testSuiteName = "notSet";
private String testSuiteVersion = "notSet";
private boolean standardBenchmarkStyleScoring = true;

private String toolName = "Unknown Tool";
private String toolVersion = null;
private String time = "Unknown"; // Scan time. e.g., '0:17:29'
public final boolean isCommercial;
public final ToolType toolType;
private Map<Integer, List<TestCaseResult>> testCaseResults =
new TreeMap<Integer, List<TestCaseResult>>();
private Map<String, List<TestCaseResult>> testCaseResults =
new TreeMap<String, List<TestCaseResult>>();

// Used to track if this tool has been anonymized
private boolean anonymous = false;
Expand Down Expand Up @@ -102,35 +103,47 @@ public boolean isCommercial() {
}

/**
* Add a test case result to the set of results for this tool.
* Add a test case result to the set of results for this tool or expected results file.
*
* @param tcr The test case result to add.
*/
public void put(TestCaseResult tcr) {

// This warning message is added just in case. It can be caused by a buggy parser or
// invalid results file.
int testCaseNum = tcr.getNumber();
if ((testCaseNum <= 0 || testCaseNum > 10000)
&& testCaseNum != TestCaseResult.NOT_USING_TESTCASE_NUMBERS) {
System.out.println(
"WARNING: Did you really intend to add a test case result for test case: "
+ testCaseNum);
String testCaseKey;

// If we are using test case numbers, we add each result to that specific test case number
if (this.standardBenchmarkStyleScoring
&& (testCaseNum != TestCaseResult.NOT_USING_TESTCASE_NUMBERS)) {
// This warning message is added just in case. It can be caused by a buggy parser or
// invalid results file.
if ((testCaseNum <= 0 || testCaseNum > 10000)) {
System.out.println(
"WARNING: Did you really intend to add a test case result for test case: "
+ testCaseNum);
}

testCaseKey = String.valueOf(testCaseNum);
} else {
// otherwise use test case names as the key, and we add each result by test case name
testCaseKey = tcr.getTestCaseName();
this.standardBenchmarkStyleScoring = false;
}

// There is a list of results for each test case
List<TestCaseResult> results = testCaseResults.get(testCaseNum);
List<TestCaseResult> results = testCaseResults.get(testCaseKey);
if (results == null) {
// If there are no results yet for this test case, create a List.
// Add this list for this test case to the set of results
// Add this entry for this test case to the set of results
results = new ArrayList<TestCaseResult>();
testCaseResults.put(tcr.getNumber(), results);
testCaseResults.put(testCaseKey, results);
}

// Add this specific result to this test case's results
results.add(tcr);
}

public List<TestCaseResult> get(int tn) {
public List<TestCaseResult> get(String tn) {
return testCaseResults.get(tn);
}

Expand All @@ -139,7 +152,7 @@ public List<TestCaseResult> get(int tn) {
*
* @return The Set of Keys.
*/
public Set<Integer> keySet() {
public Set<String> keySet() {
return testCaseResults.keySet();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public static TestSuiteResults parse(ResultFile resultFile) throws IOException {
try (final CSVParser parser = resultFile.csvRecords()) {
setResultsMetadata(parser, tr);

String testCaseName = tr.getTestSuiteName() + BenchmarkScore.TEST;
final String TESTCASENAME = tr.getTestSuiteName() + BenchmarkScore.TEST;

for (CSVRecord record : parser) {
TestCaseResult tcr = new TestCaseResult();
Expand All @@ -70,9 +70,10 @@ public static TestSuiteResults parse(ResultFile resultFile) throws IOException {
}
if (record.get(TEST_NAME)
.trim()
.startsWith(tr.getTestSuiteName() + BenchmarkScore.TEST)) {
tcr.setNumber(testNumber(record.get(TEST_NAME).trim(), testCaseName));
} else tcr.setNumber(TestCaseResult.NOT_USING_TESTCASE_NUMBERS);
.startsWith(tr.getTestSuiteName() + BenchmarkScore.TEST))
tcr.setNumber(testNumber(record.get(TEST_NAME).trim(), TESTCASENAME));
else tcr.setNumber(TestCaseResult.NOT_USING_TESTCASE_NUMBERS);

if (isExtendedResultsFile(parser)) {
tcr.setSource(record.get(SOURCE).trim());
tcr.setDataFlow(record.get(DATA_FLOW).trim());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public String createFor(TestSuiteResults actual) {
}

private boolean isFullDetails(TestSuiteResults actual) {
Iterator<Integer> iterator = actual.keySet().iterator();
Iterator<String> iterator = actual.keySet().iterator();

return iterator.hasNext() && (actual.get(iterator.next()).get(0).getSource() != null);
}
Expand All @@ -94,12 +94,12 @@ private void writeHeader(PrintStream ps, boolean fullDetails, String testSuiteVe
}

private void appendRow(
PrintStream ps, TestSuiteResults actual, Integer testNumber, boolean fullDetails) {
TestCaseResult actualResult = actual.get(testNumber).get(0);
PrintStream ps, TestSuiteResults actual, String testcaseID, boolean fullDetails) {
TestCaseResult actualResult = actual.get(testcaseID).get(0);
boolean isReal = actualResult.isTruePositive();
boolean passed = actualResult.isPassed();

ps.print(actualResult.getName());
ps.print(actualResult.getTestCaseName());
ps.print(", " + actualResult.getCategory());
ps.print(", " + actualResult.getCWE());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -371,9 +371,8 @@ protected void run() {
// 3. Calculate which codeblocks that tool seems to support and does not support

// 3a. Loop through all the results in theToolResults to calculate the initial
// statistics
// across all of them.
for (int tc : theToolResults.keySet()) {
// statistics across all of them.
for (String tc : theToolResults.keySet()) {
TestCaseResult theResult = theToolResults.get(tc).get(0); // Always only one.
boolean passed = theResult.isPassed();
CodeBlockSupportResults source = sourceCodeBlocksResults.get(theResult.getSource());
Expand All @@ -393,9 +392,8 @@ protected void run() {
// If a TP passes, we 'assume' that all code block elements are supported

// However, sources can be false positives, but the dataflow introduces
// taint, so we
// only 'know' if a source is supported if the source is also a true
// positive.
// taint, so we only 'know' if a source is supported if the source is
// also a true positive.
if (source.truePositive) source.supported = true;
source.numTPTestCasesPassed++;
dataflow.supported = true;
Expand Down Expand Up @@ -434,7 +432,7 @@ protected void run() {
// SOURCEs/DATAFLOWs: Calculate which sources cause TPs to NOT be detected.
// Loop through them all again and calculate the number of TPs for each source that
// pass/fail, ignoring any failures that are caused by unsupported SINKs.
for (int tc : theToolResults.keySet()) {
for (String tc : theToolResults.keySet()) {
TestCaseResult theResult = theToolResults.get(tc).get(0); // Always only one.
boolean passed = theResult.isPassed();
CodeBlockSupportResults source = sourceCodeBlocksResults.get(theResult.getSource());
Expand Down Expand Up @@ -485,7 +483,7 @@ protected void run() {
// Check Failures where the sink is 'supported' or hasn't been reported as a False
// Positive and the dataflow is 'null'.
boolean foundFPorFN = false;
for (int tc : theToolResults.keySet()) {
for (String tc : theToolResults.keySet()) {
TestCaseResult theResult = theToolResults.get(tc).get(0); // Always only one.
boolean passed = theResult.isPassed();
CodeBlockSupportResults source = sourceCodeBlocksResults.get(theResult.getSource());
Expand Down Expand Up @@ -592,7 +590,7 @@ protected void run() {
// Print out codeblock coordinates of the rest of the False Positives, ignoring all with
// sinks or sources already known to cause FPs
int FPCount = 1;
for (int tc : theToolResults.keySet()) {
for (String tc : theToolResults.keySet()) {
TestCaseResult theResult = theToolResults.get(tc).get(0); // Always only one.
boolean passed = theResult.isPassed();
CodeBlockSupportResults source = sourceCodeBlocksResults.get(theResult.getSource());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ void readerHandlesGivenResultFile() throws Exception {

assertEquals(2, result.getTotalResults());

assertEquals(CweNumber.COMMAND_INJECTION, result.get(1).get(0).getCWE());
assertEquals(CweNumber.XSS, result.get(2).get(0).getCWE());
assertEquals(CweNumber.COMMAND_INJECTION, result.get("1").get(0).getCWE());
assertEquals(CweNumber.XSS, result.get("2").get(0).getCWE());

// For Acunetix WVS
reader = new AcunetixReader();
Expand All @@ -70,7 +70,7 @@ void readerHandlesGivenResultFile() throws Exception {

assertEquals(2, result.getTotalResults());

assertEquals(CweNumber.LDAP_INJECTION, result.get(44).get(0).getCWE());
assertEquals(CweNumber.SQL_INJECTION, result.get(2629).get(0).getCWE());
assertEquals(CweNumber.LDAP_INJECTION, result.get("44").get(0).getCWE());
assertEquals(CweNumber.SQL_INJECTION, result.get("2629").get(0).getCWE());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ void readerHandlesGivenResultFile() throws Exception {

assertEquals(2, result.getTotalResults());

assertEquals(CweNumber.XSS, result.get(1).get(0).getCWE());
assertEquals(CweNumber.XSS, result.get(2).get(0).getCWE());
assertEquals(CweNumber.XSS, result.get("1").get(0).getCWE());
assertEquals(CweNumber.XSS, result.get("2").get(0).getCWE());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ void readerHandlesGivenResultFileInV1_30() throws Exception {

assertEquals(3, result.getTotalResults());

assertEquals(CweNumber.COMMAND_INJECTION, result.get(7).get(0).getCWE());
assertEquals(CweNumber.WEAK_HASH_ALGO, result.get(5).get(0).getCWE());
assertEquals(CweNumber.WEAK_CRYPTO_ALGO, result.get(35).get(0).getCWE());
assertEquals(CweNumber.COMMAND_INJECTION, result.get("7").get(0).getCWE());
assertEquals(CweNumber.WEAK_HASH_ALGO, result.get("5").get(0).getCWE());
assertEquals(CweNumber.WEAK_CRYPTO_ALGO, result.get("35").get(0).getCWE());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ void readerHandlesGivenResultFile() throws Exception {

assertEquals(2, result.getTotalResults());

assertEquals(CweNumber.COMMAND_INJECTION, result.get(1).get(0).getCWE());
assertEquals(CweNumber.SQL_INJECTION, result.get(2).get(0).getCWE());
assertEquals(CweNumber.COMMAND_INJECTION, result.get("1").get(0).getCWE());
assertEquals(CweNumber.SQL_INJECTION, result.get("2").get(0).getCWE());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ void readerHandlesGivenResultFile() throws Exception {

assertEquals(2, result.getTotalResults());

assertEquals(CweNumber.COMMAND_INJECTION, result.get(1).get(0).getCWE());
assertEquals(CweNumber.SQL_INJECTION, result.get(2).get(0).getCWE());
assertEquals(CweNumber.COMMAND_INJECTION, result.get("1").get(0).getCWE());
assertEquals(CweNumber.SQL_INJECTION, result.get("2").get(0).getCWE());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ void readerHandlesGivenResultFile() throws Exception {

assertEquals(2, result.getTotalResults());

assertEquals(CweNumber.SQL_INJECTION, result.get(1).get(0).getCWE());
assertEquals(CweNumber.PATH_TRAVERSAL, result.get(2).get(0).getCWE());
assertEquals(CweNumber.SQL_INJECTION, result.get("1").get(0).getCWE());
assertEquals(CweNumber.PATH_TRAVERSAL, result.get("2").get(0).getCWE());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ void readerHandlesGivenResultFile() throws Exception {

assertEquals(2, result.getTotalResults());

assertEquals(CweNumber.XSS, result.get(1).get(0).getCWE());
assertEquals(CweNumber.SQL_INJECTION, result.get(2).get(0).getCWE());
assertEquals(CweNumber.XSS, result.get("1").get(0).getCWE());
assertEquals(CweNumber.SQL_INJECTION, result.get("2").get(0).getCWE());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ void readerHandlesGivenResultFile() throws Exception {

assertEquals(2, result.getTotalResults());

assertEquals(CweNumber.PATH_TRAVERSAL, result.get(1).get(0).getCWE());
assertEquals(CweNumber.SQL_INJECTION, result.get(2).get(0).getCWE());
assertEquals(CweNumber.PATH_TRAVERSAL, result.get("1").get(0).getCWE());
assertEquals(CweNumber.SQL_INJECTION, result.get("2").get(0).getCWE());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ void readerHandlesGivenResultFile() throws Exception {

assertEquals(4, result.getTotalResults());

assertEquals(CweNumber.COMMAND_INJECTION, result.get(1609).get(0).getCWE());
assertEquals(CweNumber.PATH_TRAVERSAL, result.get(2).get(0).getCWE());
assertEquals(CweNumber.WEAK_HASH_ALGO, result.get(1).get(0).getCWE());
assertEquals(CweNumber.TRUST_BOUNDARY_VIOLATION, result.get(4).get(0).getCWE());
assertEquals(CweNumber.COMMAND_INJECTION, result.get("1609").get(0).getCWE());
assertEquals(CweNumber.PATH_TRAVERSAL, result.get("2").get(0).getCWE());
assertEquals(CweNumber.WEAK_HASH_ALGO, result.get("1").get(0).getCWE());
assertEquals(CweNumber.TRUST_BOUNDARY_VIOLATION, result.get("4").get(0).getCWE());
}
}
Loading

0 comments on commit d1aa364

Please sign in to comment.