More enhancements and additional rule tuning for a number of parsers.

Author: Dave Wichers

library/src/main/resources/categories.xml

@@ -60,6 +60,7 @@
         <id>deserialization</id>
         <name>Insecure Deserialization</name>
         <cwe>502</cwe>
+        <childof>913,915</childof>
         <isInjection>false</isInjection>
         <shortname>DESER</shortname>
     </category>
@@ -131,12 +132,14 @@
         <id>redirect</id>
         <name>Unchecked Redirect</name>
         <cwe>601</cwe>
+        <childof>610</childof>
         <shortname>REDIR</shortname>
     </category>
     <category>
         <id>reflecti</id>
         <name>Unsafe Reflection</name>
         <cwe>470</cwe>
+        <childof>610,913</childof>
         <isInjection>true</isInjection>
         <shortname>REFLECT</shortname>
     </category>
@@ -160,6 +163,7 @@
         <id>trustbound</id>
         <name>Trust Boundary</name>
         <cwe>501</cwe>
+        <childof>664</childof>
         <shortname>TRBO</shortname>
     </category>
     <category>
@@ -189,14 +193,16 @@
         <id>xss</id>
         <name>XSS (Cross-Site Scripting)</name>
         <cwe>79</cwe>
-        <parentof>80,81,83,84,85,86,87</parentof>
+        <parentof>80,81,82,83,84,85,86,87</parentof>
         <!-- isInjection>Don't set. Not a classic injection issue.</isInjection -->
         <shortname>XSS</shortname>
+        <note>82 added manually as its the child of 83.</note>
     </category>
     <category>
         <id>xxe</id>
         <name>XML eXternal Entity Injection</name>
         <cwe>611</cwe>
+        <childof>610</childof>
         <isInjection>true</isInjection>
         <shortname>XXE</shortname>
     </category>

plugin/src/main/java/org/owasp/benchmarkutils/helpers/Utils.java

@@ -239,7 +239,7 @@ public static void copyFilesFromDirRecursively(String source, final Path target)
             return;
         }
 
-        if (srcURL.getProtocol().equals("file")) {
+        if ("file".equals(srcURL.getProtocol())) {
             // Copy the files from one directory to another using the normal
             // FileUtils.copyDirectory() method
 
@@ -278,7 +278,7 @@ public static void copyFilesFromDirRecursively(String source, final Path target)
             return; // File(s) copied or it failed
         }
 
-        if (!srcURL.getProtocol().equals("jar")) {
+        if (!"jar".equals(srcURL.getProtocol())) {
             System.err.printf(
                     "ERROR: source resource not a file: or jar: resource. It is: %s%n", srcURL);
             return;

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

@@ -88,9 +88,6 @@ public class BenchmarkScore extends AbstractMojo {
     // scorecard dir normally created under current user directory
     public static final String SCORECARDDIRNAME = "scorecard";
 
-    // The values stored in this is pulled from the categories.xml config file
-    //    public static Categories CATEGORIES;
-
     /*
      * The set of all the Tools. Each Tool includes the results for that tool.
      */
@@ -416,7 +413,7 @@ public static void main(String[] args) {
 
             // catch try for Steps 4 & 5
         } catch (Exception e) {
-            System.out.println("Error during processing: " + e.getMessage());
+            System.err.println("Error during processing: " + e.getMessage());
             e.printStackTrace();
         }
 
@@ -526,7 +523,7 @@ public static void main(String[] args) {
 
             Files.write(homeFilePath, html.getBytes());
         } catch (IOException e) {
-            System.out.println("Error updating results table in: " + homeFilePath.getFileName());
+            System.err.println("Error updating results table in: " + homeFilePath.getFileName());
             e.printStackTrace();
         }
 
@@ -535,7 +532,7 @@ public static void main(String[] args) {
         try {
             scatter.writeChartToFile(new File(scoreCardDir, "content/testsuite_guide.png"), 800);
         } catch (IOException e) {
-            System.out.println(
+            System.err.println(
                     "ERROR: Couldn't create content/testsuite_guide.png file for some reason.");
             e.printStackTrace();
         }
@@ -608,13 +605,13 @@ private static void process(
                 // printExtraCWE( expectedResults, actualResults );
             } else {
                 if (expectedResults == null) {
-                    System.out.println("Error!!: expected results were null.");
+                    System.err.println("Error!!: expected results were null.");
                 } else
-                    System.out.println(
+                    System.err.println(
                             "Error!!: actual results were null for file: " + rawToolResultsFile);
             }
         } catch (Exception e) {
-            System.out.println("Error processing " + rawToolResultsFile + ". Continuing.");
+            System.err.println("Error processing " + rawToolResultsFile + ". Continuing.");
             e.printStackTrace();
         }
     }
@@ -858,7 +855,7 @@ private static CweMatchDetails compare(
         // return true if there are no actual results and this was a false positive test
         if (actList == null || actList.isEmpty()) {
             return new CweMatchDetails(
-                    expectedCWE, exp.isTruePositive(), !exp.isTruePositive(), -1, "");
+                    expectedCWE, exp.isTruePositive(), !exp.isTruePositive(), -1, "", actList);
         }
 
         // Check actual results for an exact CWE match.
@@ -872,7 +869,12 @@ private static CweMatchDetails compare(
             // immediately return a value if we find an exact match
             if (actualCWE == expectedCWE) {
                 return new CweMatchDetails(
-                        expectedCWE, exp.isTruePositive(), exp.isTruePositive(), actualCWE, "");
+                        expectedCWE,
+                        exp.isTruePositive(),
+                        exp.isTruePositive(),
+                        actualCWE,
+                        "",
+                        actList);
             }
         }
 
@@ -887,20 +889,22 @@ private static CweMatchDetails compare(
                         exp.isTruePositive(),
                         exp.isTruePositive(),
                         actualCWE,
-                        "ChildOf");
+                        "ChildOf",
+                        actList);
             }
             if (expectedCWECategory.isParentOf(actualCWE)) {
                 return new CweMatchDetails(
                         expectedCWE,
                         exp.isTruePositive(),
                         exp.isTruePositive(),
                         actualCWE,
-                        "ParentOf");
+                        "ParentOf",
+                        actList);
             }
         }
         // if we couldn't find a match, then return true if it's a False Positive test
         return new CweMatchDetails(
-                expectedCWE, exp.isTruePositive(), !exp.isTruePositive(), -1, "");
+                expectedCWE, exp.isTruePositive(), !exp.isTruePositive(), -1, "", actList);
     }
 
     // Create a TestResults object that contains the expected results for this version

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

@@ -77,6 +77,13 @@ public class Configuration {
     /** Indicates whether Precision score should be included in generated tables. By default, no. */
     public final boolean includePrecision;
 
+    /**
+     * This is a debug flag, which if set in yaml config file, causes all the CSVs for each test
+     * case to be included in the CSV results file if no CSV matches the expected result for that
+     * test case.
+     */
+    public static boolean includeAllCWEsInCSVFile = false;
+
     public final Report report;
 
     private static final Yaml yaml = new Yaml();
@@ -163,6 +170,10 @@ private Configuration(Map<String, Object> yamlConfig) {
         includeProjectLink = (Boolean) yamlConfig.get("includeprojectlink");
         includePrecision = (Boolean) yamlConfig.get("includeprecision");
 
+        // Optional config parameter for debugging/testing only
+        if (yamlConfig.get("includecwesincsvresults") != null)
+            includeAllCWEsInCSVFile = (Boolean) yamlConfig.get("includecwesincsvresults");
+
         report = new Report(yamlConfig);
     }
 

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

@@ -17,6 +17,8 @@
  */
 package org.owasp.benchmarkutils.score;
 
+import java.util.List;
+
 /**
  * This class contains the details of how a tool's result matches to a particular test case. With
  * CWE parent/child matching, we want to record exactly how a tool's finding matched. Was it an
@@ -39,18 +41,22 @@ public class CweMatchDetails {
     public final int actualCWEreported; // -1 if not found
     // Empty string if exact match or no match. ChildOF if a child CWE, ParentOf if parent CWE
     public final String CWErelationship;
+    // The full set of findings for this test case
+    public final List<TestCaseResult> actList;
 
     public CweMatchDetails(
             int expectedCWE,
             boolean truePositive,
             boolean pass,
             int actualCWEreported,
-            String CWErelationship) {
+            String CWErelationship,
+            List<TestCaseResult> actList) {
         this.expectedCWE = expectedCWE;
         this.truePositive = truePositive;
         this.pass = pass;
         this.actualCWEreported = actualCWEreported; // Suggest -1 if not found
         // Blank string if not found or exact CWE match
         this.CWErelationship = (CWErelationship == null ? "" : CWErelationship);
+        this.actList = actList;
     }
 }

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

@@ -17,10 +17,10 @@
  */
 package org.owasp.benchmarkutils.score;
 
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
 import java.io.File;
+import java.io.FileInputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.StringReader;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
@@ -46,6 +46,10 @@ public class ResultFile {
     private final File originalFile;
     private JSONObject contentAsJson;
     private Document contentAsXml;
+    // This is a special stream only set by test cases, because they are resource files, not actual
+    // files on the file system. It is only used by the extract() method to pull files out of ZIP
+    // archives in test case. The normal code pulls the data out of the result file.
+    InputStream streamToFile = null; // If null, not used.
 
     public ResultFile(File fileToParse) throws IOException {
         this(fileToParse, readFileContent(fileToParse));
@@ -61,8 +65,8 @@ public ResultFile(String filename, byte[] rawContent) throws IOException {
 
     public ResultFile(File fileToParse, byte[] rawContent) throws IOException {
         this.rawContent = rawContent;
-        originalFile = fileToParse;
-        filename = originalFile.getName();
+        this.originalFile = fileToParse;
+        this.filename = originalFile.getName();
         parseJson();
         parseXml();
     }
@@ -191,16 +195,29 @@ public String xmlRootNodeName() {
     }
 
     /**
-     * Extracts a file from a packed ResultFile.
+     * Finds the specified file in the zip file associated with this ResultFile, and returns an
+     * InputStream to the specified file.
      *
-     * @return
+     * @return An InputStream to the specified file.
      */
-    public ResultFile extract(String zipPath) {
-        try (ZipInputStream zipIn = new ZipInputStream(new ByteArrayInputStream(rawContent))) {
+    public InputStream extract(String zipPath) {
+        try {
+            ZipInputStream zipIn;
+            // Check to see if a stream to the file was set by a test case. If so, use that instead
+            // of the File reference.
+            if (this.streamToFile != null) zipIn = new ZipInputStream(this.streamToFile);
+            else zipIn = new ZipInputStream(new FileInputStream(this.originalFile));
+
             ZipEntry entry = zipIn.getNextEntry();
             while (entry != null) {
                 if (entry.getName().equals(zipPath)) {
-                    return readFileFromZip(zipPath, zipIn);
+                    // NOTE: Previously this method used to call another method that extracted the
+                    // file into a new ResultFile with just the ZIP file attached to it. However,
+                    // for VERY large Fortify files, you couldn't create a byte[] big enough to hold
+                    // the entire file (as its was > 2GB), so you got an OutOfMemoryError. So now we
+                    // return a Stream instead, and that gets passed to the DOM Parser directly,
+                    // which works fine for large files.
+                    return zipIn;
                 }
                 zipIn.closeEntry();
                 entry = zipIn.getNextEntry();
@@ -211,15 +228,4 @@ public ResultFile extract(String zipPath) {
 
         throw new RuntimeException("ZipFile does not contain " + zipPath);
     }
-
-    private ResultFile readFileFromZip(String zipPath, ZipInputStream zipIn) throws IOException {
-        try (ByteArrayOutputStream bos = new ByteArrayOutputStream()) {
-            final byte[] buf = new byte[1024];
-            int length;
-            while ((length = zipIn.read(buf, 0, buf.length)) >= 0) {
-                bos.write(buf, 0, length);
-            }
-            return new ResultFile(zipPath, bos.toByteArray());
-        }
-    }
 }

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

@@ -263,7 +263,7 @@ public String getMatchingTestCaseName(String testCaseFilename) {
      * @param filename
      * @return The filename without any path info (if any)
      */
-    private String getFileNameNoPath(String filename) {
+    public static String getFileNameNoPath(String filename) {
         // We look for / and \ and strip off everything before and including the path separator. We
         // check both path chars because the results could be generated on one platform and scored
         // on another with different path separators.

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

@@ -178,7 +178,7 @@ private static String getLine(File file, String toFind, boolean nextLine) {
 
         try (BufferedReader br = new BufferedReader(new FileReader(file))) {
             String line = "";
-            while (line.equals("")) {
+            while ("".equals(line)) {
                 line = br.readLine();
                 if (line.contains(toFind)) {
                     if (nextLine) return br.readLine();
@@ -208,7 +208,7 @@ private static void listPathFiles(String path) {
     }*/
 
     void deletePreviousResults(String toolName, String toolVersion, String testSuiteVersion) {
-        if (!toolName.equals("")) {
+        if (!"".equals(toolName)) {
             File targetDir = new File("results/");
             if (targetDir.exists()) {
                 File[] files = targetDir.listFiles();