refactor AppScanReader to use provided CWE (#93)

Author: darkspirit510

plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/HCLAppScanStandardReader.java

@@ -17,26 +17,48 @@
  */
 package org.owasp.benchmarkutils.score.parsers;
 
-import java.io.StringReader;
+import static java.lang.Integer.parseInt;
+
 import java.util.ArrayList;
 import java.util.List;
-import javax.xml.parsers.DocumentBuilder;
-import javax.xml.parsers.DocumentBuilderFactory;
 import org.owasp.benchmarkutils.score.BenchmarkScore;
 import org.owasp.benchmarkutils.score.CweNumber;
 import org.owasp.benchmarkutils.score.ResultFile;
 import org.owasp.benchmarkutils.score.TestCaseResult;
 import org.owasp.benchmarkutils.score.TestSuiteResults;
-import org.w3c.dom.Document;
-import org.w3c.dom.NamedNodeMap;
 import org.w3c.dom.Node;
-import org.xml.sax.InputSource;
 
-// This is the new HCL AppScan DAST reader, where they generate ".xml" files. HCL calls this AppScan
-// Standard.
-// The 'old' reader is AppScanDynamicReader, which supports the previous .xml format from IBM.
+/**
+ * This is the new HCL AppScan DAST reader, where they generate ".xml" files. HCL calls this AppScan
+ * Standard. The 'old' reader is AppScanDynamicReader, which supports the previous .xml format from
+ * IBM.
+ */
 public class HCLAppScanStandardReader extends Reader {
 
+    private final List<String> ignoreList = new ArrayList<>();
+
+    public HCLAppScanStandardReader() {
+        ignoreList.add("attContentSecurityPolicyObjectSrc");
+        ignoreList.add("attContentSecurityPolicyScriptSrc");
+        ignoreList.add("attCachedSSL");
+        ignoreList.add("attJSCookie");
+        ignoreList.add("attLinkInjection");
+        ignoreList.add("attUndefinedState");
+        ignoreList.add("bodyParamsInQuery");
+        ignoreList.add("ContentSecurityPolicy");
+        ignoreList.add("ContentTypeOptions");
+        ignoreList.add("GD_EmailAddress");
+        ignoreList.add("GETParamOverSSL");
+        ignoreList.add("GV_SQLErr");
+        ignoreList.add("HSTS");
+        ignoreList.add("MHTMLXSS");
+        ignoreList.add("OpenSource");
+        ignoreList.add("phishingInFrames");
+        ignoreList.add("OldTLS");
+        ignoreList.add("ShellShockCheck");
+        ignoreList.add("SriSupport");
+    }
+
     @Override
     public boolean canRead(ResultFile resultFile) {
         return resultFile.filename().endsWith(".xml")
@@ -47,245 +69,75 @@ public boolean canRead(ResultFile resultFile) {
 
     @Override
     public TestSuiteResults parse(ResultFile resultFile) throws Exception {
-        DocumentBuilderFactory docBuilderFactory = DocumentBuilderFactory.newInstance();
-        // Prevent XXE
-        docBuilderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
-        DocumentBuilder docBuilder = docBuilderFactory.newDocumentBuilder();
-        InputSource is = new InputSource(new StringReader(resultFile.content()));
-        Document doc = docBuilder.parse(is);
-
-        Node root = doc.getDocumentElement();
-        Node scanInfo = getNamedChild("scan-information", root);
-
-        Node scanConfiguration = getNamedChild("scan-configuration", root);
-        String startingUrl = getNamedChild("starting-url", scanConfiguration).getTextContent();
+        Node root = resultFile.xml().getDocumentElement();
 
         TestSuiteResults tr =
                 new TestSuiteResults("HCL AppScan Standard", true, TestSuiteResults.ToolType.DAST);
 
-        // version is usually like 9.3.0 but sometimes like 9.3.0 iFix005. We trim off the part
-        // after the space char.
-        Node version = getNamedChild("product-version", scanInfo);
-        //    System.out.println("Product version is: " + version.getTextContent());
-        if (version != null) {
-            tr.setToolVersion(version.getTextContent().split(" ")[0]);
-        }
-        
-        Node allIssueVariants = getNamedChild("issue-group", root);
-        List<Node> variants = getNamedChildren("item", allIssueVariants);
-        
-        List<String> testCaseElementsFromVariants = new ArrayList<>();
-        
-        
-        if (variants.isEmpty()) {
-            // Handle non-variant issue types , Older xml format as in 9.x release versions and
-            // before
-            // First get the type of vuln, and if we don't care about that type, move on
-        	 Node allIssues = getNamedChild("url-group", root);
-        	 List<Node> vulnerabilities = getNamedChildren("item", allIssues);
-        	 for (Node vulnerability : vulnerabilities) {
-                 String issueType = getNamedChild("issue-type", vulnerability).getTextContent();
-
-                 String url = getNamedChild("name", vulnerability).getTextContent();
-                 
-            TestCaseResult tcr = TestCaseLookup(issueType, url);
-            if (tcr != null) tr.put(tcr);
-        	 }
-        }
-        
-        else {
-            // Handle issues which are Variants, new xml format after 10.x release
-        	for (Node variant : variants) {
-	            String variantIssueType = getNamedChild("issue-type", variant).getTextContent().trim();
-	            // System.out.println("Variant Url Ref ID: " + variantUrlRefId);
-	
-	            // Add the record only if the issue type matches for the relevant variants
-	                Node variantNodes = getNamedChild("variant-group", variant);
-	                List<Node> variantNodeChildren = getNamedChildren("item", variantNodes);
-	                for (Node variantNodeChild : variantNodeChildren) {
-	                    String httpTraffic =
-	                            getNamedChild("test-http-traffic", variantNodeChild).getTextContent();
-	                    String[] variantUrl = httpTraffic.split(" ");
-	
-	                    String benchMarkTestCase = variantUrl[1].trim();
-	
-	                    if (benchMarkTestCase.contains("BenchmarkTest")) {
-	                        String[] urlElements = benchMarkTestCase.split("/");
-	
-	                        String testAreaUrl =
-	                                startingUrl
-	                                        + urlElements[urlElements.length - 2]
-	                                        + "/"
-	                                        + urlElements[urlElements.length - 1];
-	                        String testArea = testAreaUrl.split("\\?")[0]; // .split strips off the -##
-	
-	                        if (testArea.contains("BenchmarkTest"))
-	                            testCaseElementsFromVariants.add(testArea);
-			                    TestCaseResult tcr = TestCaseLookup(variantIssueType, testArea);
-			                    if (tcr != null) 
-			                    	tr.put(tcr);
-	                    }
-	                }
-            }
-        }
-        return tr;
-    }
-
-    /// Issues which are not variants
-    private TestCaseResult TestCaseLookup(String issueType, String url) {
-        String[] urlElements = url.split("/");
-        String testArea =
-                urlElements[urlElements.length - 2].split("-")[0]; // .split strips off the -##
-
-        int vtype = cweLookup(issueType, testArea);
-
-        // Then get the filename containing the vuln. And if not in a test case, skip it.
-        // Parse out test number from:
-        // https://localhost:port/benchmark/testarea-##/BenchmarkTest02603
-        int startOfTestCase = url.lastIndexOf("/") + 1;
-        String testcase = url.substring(startOfTestCase, url.length());
-        // if test case has extension (e.g., BenchmarkTestCase#####.html), strip it off.
-        testcase = testcase.split("\\.")[0];
-        // System.out.println("Candidate test case is: " + testcase);
-        if (testcase.startsWith(BenchmarkScore.TESTCASENAME)) {
-            // if (tn == -1) System.out.println("Found vuln outside of test case of type: " +
-            // issueType);
-
-            // Add the vuln found in a test case to the results for this tool
-            TestCaseResult tcr = new TestCaseResult();
-            tcr.setNumber(testNumber(testcase));
-            tcr.setCategory(issueType); // TODO: Is this right?
-            tcr.setCWE(vtype);
-            tcr.setEvidence(issueType);
-            return tcr;
-        }
-        return null;
-    }
-
-    // Fetch Issues listed as variants, to cater to post 10.x release xml format
-    private List<String> variantLookup(
-            String issueType, String itemID, String startingUrl, List<Node> variants) {
-        List<String> testCaseElementsFromVariants = new ArrayList<>();
+        setTime(root, tr);
+        setVersion(root, tr);
 
-        // System.out.println("Variant Lookup Item ID: " + itemID);
+        List<Node> variants = getNamedChildren("item", getNamedChild("issue-group", root));
 
         for (Node variant : variants) {
-            String variantUrlRefId = getNamedChild("url", variant).getTextContent().trim();
+            int xmlCwe = parseInt(getNamedChild("cwe", variant).getTextContent());
             String variantIssueType = getNamedChild("issue-type", variant).getTextContent().trim();
-            // System.out.println("Variant Url Ref ID: " + variantUrlRefId);
-
-            // Add the record only if the issue type matches for the relevant variants
-            if (issueType.equals(variantIssueType) && itemID.equals(variantUrlRefId)) {
-                Node variantNodes = getNamedChild("variant-group", variant);
-                List<Node> variantNodeChildren = getNamedChildren("item", variantNodes);
-                for (Node variantNodeChild : variantNodeChildren) {
-                    String httpTraffic =
-                            getNamedChild("test-http-traffic", variantNodeChild).getTextContent();
-                    String[] variantUrl = httpTraffic.split(" ");
-
-                    String benchMarkTestCase = variantUrl[1].trim();
 
-                    if (benchMarkTestCase.contains("BenchmarkTest")) {
-                        String[] urlElements = benchMarkTestCase.split("/");
+            getNamedChildren("item", getNamedChild("variant-group", variant)).stream()
+                    .map(node -> extractFilenameWithoutEnding(extractUrlFrom(node)))
+                    .filter(filename -> filename.startsWith(BenchmarkScore.TESTCASENAME))
+                    .forEach(
+                            filename -> {
+                                TestCaseResult tcr = new TestCaseResult();
 
-                        String testAreaUrl =
-                                startingUrl
-                                        + urlElements[urlElements.length - 2]
-                                        + "/"
-                                        + urlElements[urlElements.length - 1];
-                        String testArea = testAreaUrl.split("\\?")[0]; // .split strips off the -##
+                                tcr.setNumber(testNumber(filename));
+                                tcr.setCategory(variantIssueType); // TODO: Is this right?
+                                tcr.setCWE(cweLookup(variantIssueType, xmlCwe));
+                                tcr.setEvidence(variantIssueType);
 
-                        if (testArea.contains("BenchmarkTest"))
-                            testCaseElementsFromVariants.add(testArea);
-                    }
-                }
-            }
+                                tr.put(tcr);
+                            });
         }
 
-        return testCaseElementsFromVariants;
+        return tr;
     }
 
-    private int cweLookup(String vtype, String testArea) {
-        int cwe = cweLookup(vtype); // Do the standard CWE lookup
-
-        // Then map some to other CWEs based on the test area being processed.
-        if ("xpathi".equals(testArea) && cwe == 89) cwe = 643; // CWE for XPath injection
-        if ("ldapi".equals(testArea) && cwe == 89) cwe = 90; // CWE for LDAP injection
-
-        return cwe;
+    private void setTime(Node root, TestSuiteResults tr) {
+        tr.setTime(
+                getNamedChild("scan-Duration", getNamedChild("scan-summary", root))
+                        .getTextContent());
     }
 
-    private int cweLookup(String vtype) {
-        switch (vtype) {
-            case "attDirectoryFound":
-            case "attDirOptions":
-            case "attFileUnix":
-                return CweNumber.PATH_TRAVERSAL;
-
-            case "attApplicationRemoteCodeExecutionAdns":
-            case "attCodeInjectionInSystemCall":
-            case "attCommandInjectionAdns":
-            case "attCommandInjectionUnixTws":
-            case "attFileParamPipe":
-                return CweNumber.COMMAND_INJECTION;
-
-            case "attCrossSiteScripting":
-                return CweNumber.XSS;
-
-            case "attBlindSqlInjectionStrings":
-            case "attSqlInjectionChecks":
-                return CweNumber.SQL_INJECTION;
-
-            case "attLDAPInjection":
-            case "attLDAPInjection2":
-            case "attBlindLDAPInjection":
-                return CweNumber.LDAP_INJECTION;
+    private static String extractUrlFrom(Node variantNodeChild) {
+        String[] variantUrl =
+                getNamedChild("test-http-traffic", variantNodeChild).getTextContent().split(" ");
 
-            case "SHA1CipherSuites":
-                return CweNumber.WEAK_HASH_ALGO; // Better if set to 327?
+        return variantUrl[1].trim();
+    }
 
-            case "passParamGET":
-                return CweNumber.UNPROTECTED_CREDENTIALS_TRANSPORT;
+    /*
+     * Version is usually like 9.3.0 but sometimes like 9.3.0 iFix005. We trim off the part after the space char.
+     */
+    private static void setVersion(Node root, TestSuiteResults tr) {
+        Node version = getNamedChild("product-version", getNamedChild("scan-information", root));
 
-            case "attRespCookieNotSecureSSL":
-                return CweNumber.INSECURE_COOKIE;
+        if (version != null) {
+            tr.setToolVersion(version.getTextContent().split(" ")[0]);
+        }
+    }
 
+    private int cweLookup(String vtype, int xmlCwe) {
+        switch (vtype) {
             case "attXPathInjection":
             case "attBlindXpathInjectionSingleQuote":
             case "attBlindXPathInjection":
                 return CweNumber.XPATH_INJECTION;
+        }
 
-                // These don't map to anything we care about
-            case "attContentSecurityPolicyObjectSrc":
-            case "attContentSecurityPolicyScriptSrc":
-            case "attCachedSSL":
-            case "attJSCookie":
-                // case "attLinkInjection" : return 00;
-            case "attUndefinedState":
-            case "bodyParamsInQuery":
-            case "ContentSecurityPolicy":
-            case "ContentTypeOptions":
-            case "GD_EmailAddress":
-            case "GETParamOverSSL":
-            case "GV_SQLErr":
-                // case "HSTS" : return 00;
-
-                // Microsoft MHTML XSS - Giving AppScan XSS 'credit' for this introduces ~2.4% False
-                // Positives and no real ones so I (shivababuh) disabled it instead
-            case "MHTMLXSS":
-                // case "OpenSource" : return 00;  // Known vuln in open source lib.
-                // case "phishingInFrames" : return 00;
-            case "OldTLS":
-            case "ShellShockCheck":
-            case "SriSupport":
-                // case "SSL_CertWithBadCN" : return 00;
-                // case "XSSProtectionHeader" : return 00;
-                return CweNumber.DONTCARE;
-
-            default:
-                System.out.println(
-                        "WARNING: HCL AppScan Standard-Unrecognized finding type: " + vtype);
+        if (ignoreList.contains(vtype)) {
+            return CweNumber.DONTCARE;
         }
-        return 0;
+
+        return xmlCwe;
     }
 }

plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/MendReader.java

@@ -47,7 +47,7 @@ public TestSuiteResults parse(ResultFile resultFile) throws Exception {
                 for (Report.EngineResults.Result.Vulnerability vulnerability :
                         result.vulnerabilities) {
                     try {
-                        String testfile = extractFilename(vulnerability.filename);
+                        String testfile = extractFilenameWithoutEnding(vulnerability.filename);
 
                         if (testfile.startsWith(BenchmarkScore.TESTCASENAME)) {
                             TestCaseResult tcr = new TestCaseResult();

plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/Rapid7Reader.java

@@ -47,7 +47,7 @@ public TestSuiteResults parse(ResultFile resultFile) throws Exception {
 
         for (Report.Vulnerability vulnerability : report.vulnerabilities) {
             try {
-                String testfile = extractFilename(vulnerability.url);
+                String testfile = extractFilenameWithoutEnding(vulnerability.url);
 
                 if (testfile.startsWith(BenchmarkScore.TESTCASENAME)) {
                     TestCaseResult tcr = new TestCaseResult();

plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/Reader.java

@@ -274,11 +274,15 @@ public static int testNumber(String path, String testCaseName) {
         }
     }
 
-    public static String extractFilename(String path) {
+    public static String extractFilenameWithoutEnding(String path) {
         try {
-            path = removeUrlPart(path);
+            String name = new File(fixWindowsPath(removeUrlPart(path))).getName();
 
-            return new File(fixWindowsPath(path)).getName();
+            if (name.contains(".")) {
+                return name.substring(0, name.lastIndexOf("."));
+            } else {
+                return name;
+            }
         } catch (Throwable t) {
             return "";
         }

plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/sarif/SarifReader.java

@@ -236,7 +236,7 @@ public String toolName(ResultFile resultFile) {
     private TestCaseResult testCaseResultFor(JSONObject result, Map<String, Integer> mappings) {
         TestCaseResult tcr = new TestCaseResult();
 
-        String className = extractFilename(resultUri(result));
+        String className = extractFilenameWithoutEnding(resultUri(result));
 
         if (!className.startsWith(BenchmarkScore.TESTCASENAME)) {
             return null;