Additional work in progress. Changed testcase expected/actual results

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.

Author: Dave Wichers

library/src/main/java/org/owasp/benchmarkutils/helpers/Category.java

@@ -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;

library/src/main/resources/categories.xml

@@ -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>

plugin/src/main/java/org/owasp/benchmarkutils/score/BenchmarkScore.java

@@ -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());
@@ -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);
@@ -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);

plugin/src/main/java/org/owasp/benchmarkutils/score/TestCaseResult.java

@@ -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;
     }
 

plugin/src/main/java/org/owasp/benchmarkutils/score/TestSuiteResults.java

@@ -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;
@@ -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);
     }
 
@@ -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();
     }
 

plugin/src/main/java/org/owasp/benchmarkutils/score/service/ExpectedResultsProvider.java

@@ -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();
@@ -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());

plugin/src/main/java/org/owasp/benchmarkutils/score/service/ResultsFileCreator.java

@@ -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);
     }
@@ -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());
 

plugin/src/main/java/org/owasp/benchmarkutils/tools/CalculateToolCodeBlocksSupport.java

@@ -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());
@@ -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;
@@ -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());
@@ -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());
@@ -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());

plugin/src/test/java/org/owasp/benchmarkutils/score/parsers/AcunetixReaderTest.java

@@ -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();
@@ -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());
     }
 }

plugin/src/test/java/org/owasp/benchmarkutils/score/parsers/ArachniReaderTest.java

@@ -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());
     }
 }

plugin/src/test/java/org/owasp/benchmarkutils/score/parsers/BearerReaderTest.java

@@ -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());
     }
 }

plugin/src/test/java/org/owasp/benchmarkutils/score/parsers/BurpReaderTest.java

@@ -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());
     }
 }

plugin/src/test/java/org/owasp/benchmarkutils/score/parsers/CASTAIPReaderTest.java

@@ -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());
     }
 }

plugin/src/test/java/org/owasp/benchmarkutils/score/parsers/CheckmarxIASTReaderTest.java

@@ -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());
     }
 }

plugin/src/test/java/org/owasp/benchmarkutils/score/parsers/CheckmarxReaderTest.java

@@ -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());
     }
 }

plugin/src/test/java/org/owasp/benchmarkutils/score/parsers/CoverityReaderTest.java

@@ -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());
     }
 }

plugin/src/test/java/org/owasp/benchmarkutils/score/parsers/DatadogReaderTest.java

@@ -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());
     }
 }