Add support for OWASP ZAP reports with merged alerts

Change class ZapReader to cope with reports that contain merged alerts.
Since version 2.4.2 of OWASP ZAP it's possible to create reports where
alerts of same type are merged into one (which greatly reduce the size
of the report by removing duplicated information). One single alert can
have multiple URIs and its details (param, attack and evidence).
The XML structure of reports with merged alerts is changed from:
<alert>
  <!-- other alert details (name, description, cweid...) -->
  <uri />
  <param />
  <attack />
  <evidence />
</alert>
to:
<alert>
  <!-- other alert details (name, description, cweid...) -->
  <instances>
    <instance>
      <uri />
      <param />
      <attack />
      <evidence />
    </instance>
    <!-- more "instance" elements per merged alert -->
  </instances>
</alert>
The class ZapReader now checks for the presence of "instances" element
in the alert, if it has it, it creates a TestCaseResult per "instance"
element otherwise it's created just one TestCaseResult, as before.

Author: thc202

src/main/java/org/owasp/benchmark/score/parsers/ZapReader.java

@@ -54,11 +54,7 @@ public TestResults parse(File f) throws Exception {
 
         for ( Node flaw : issueList ) {
             try {
-                TestCaseResult tcr = parseZapIssue(flaw);
-                if (tcr != null ) {
-                    // System.out.println( tcr.getNumber() + " " + tcr.getName() + " -> " + tcr.getCWE() + "\t" + tcr.getEvidence() );
-                    tr.put(tcr);
-                }
+                parseAndAddZapIssues(flaw, tr);
             } catch( Exception e ) {
                 // print and continue
                 e.printStackTrace();
@@ -80,9 +76,20 @@ public TestResults parse(File f) throws Exception {
 //      <riskdesc>Low (Medium)</riskdesc>
 //      <desc>Web Browser XSS Protection is not enabled, or is disabled by the configuration of the 'X-XSS-Protection' HTTP response header on the web server
 //        </desc>
+
 //    <uri>http://localhost:8080/benchmark/BenchmarkTest00028.html</uri>
 //      <param/>
 //      <attack/>
+// OR, for merged reports:
+//      <instances>
+//        <instance>
+//          <uri>http://localhost:8080/benchmark/BenchmarkTest00028.html</uri>
+//          <param/>
+//          <attack/>
+//        </instance>
+//        <!-- more "instance" elements per merged alert -->
+//      </instances>
+
 //      <otherinfo>The X-XSS-Protection HTTP response header allows the web server to enable or disable the web browser's XSS protection mechanism. The following values would attempt to enable it: 
 //      <solution>Ensure that the web browser's XSS filter is enabled, by setting the X-XSS-Protection HTTP response header to '1'.
 //        </solution>
@@ -98,23 +105,43 @@ public TestResults parse(File f) throws Exception {
 
 
 
-    private TestCaseResult parseZapIssue(Node flaw) throws URISyntaxException {
-        TestCaseResult tcr = new TestCaseResult();
+    private void parseAndAddZapIssues(Node flaw, TestResults tr) throws URISyntaxException {
+        int cwe = -1;
         Node rule = getNamedChild("cweid", flaw);
         if ( rule != null ) {
-            tcr.setCWE( cweLookup( rule.getTextContent() ) );
+            cwe = cweLookup( rule.getTextContent() );
         }
 
         String cat = getNamedChild("alert", flaw).getTextContent();
-        tcr.setCategory( cat );
 
-        String conf = getNamedChild( "confidence", flaw ).getTextContent();
-        tcr.setConfidence( Integer.parseInt( conf ) );
+        int conf = Integer.parseInt(getNamedChild( "confidence", flaw ).getTextContent());
+
+        Node instances = getNamedChild("instances", flaw);
+        if (instances == null) {
+            addIssue(flaw, tr, cwe, cat, conf);
+            return;
+        }
+
+        List<Node> instanceList = getNamedChildren("instance", instances);
+        for (Node instance : instanceList) {
+            addIssue(instance, tr, cwe, cat, conf);
+        }
+    }
+
+    private void addIssue(Node alertData, TestResults tr, int cwe, String category, int confidence) throws URISyntaxException {
+        int testNumber = extractTestNumber(getNamedChild("uri", alertData).getTextContent());
+        if (testNumber != -1) {
+            TestCaseResult tcr = createTestCaseResult(cwe, category, confidence, testNumber);
+            // System.out.println( tcr.getNumber() + " " + tcr.getName() + " -> " + tcr.getCWE() + "\t" + tcr.getEvidence() );
+            tr.put(tcr);
+        }
+    }
 
-        tcr.setEvidence( cat );
+    private static int extractTestNumber(String uri) throws URISyntaxException {
+        // Remove the query and fragment from the URI because some of alert URIs (e.g. generated by DOM XSS) might be malformed
+        // (characters that should be escaped are not) which leads to exceptions when parsed by java.net.URI.
+        URI url = new URI(removeQueryAndFragment(uri));
 
-        String uri = getNamedChild( "uri", flaw ).getTextContent();
-        URI url = new URI( uri );
         String testfile = url.getPath();
         testfile = testfile.substring( testfile.lastIndexOf('/') +1 );
 
@@ -124,15 +151,38 @@ private TestCaseResult parseZapIssue(Node flaw) throws URISyntaxException {
                 testno = testno.substring(0, testno.length() -5 );
             }
             try {
-                tcr.setNumber( Integer.parseInt( testno ) );
-                return tcr;
+                return Integer.parseInt( testno );
             } catch( NumberFormatException e ) {
                 System.out.println( "> Parse error " + testfile + ":: " + testno );
             }
         }
-        return null;
+        return -1;
     }
 
+    private static String removeQueryAndFragment(String uri) {
+        String strippedUri = uri;
+        int idx = strippedUri.indexOf('?');
+        if (idx != -1) {
+            strippedUri = strippedUri.substring(0, idx);
+        }
+        idx = strippedUri.indexOf('#');
+        if (idx != -1) {
+            strippedUri = strippedUri.substring(0, idx);
+        }
+        return strippedUri;
+    }
+
+    private static TestCaseResult createTestCaseResult(int cwe, String category, int confidence, int testNumber) {
+        TestCaseResult tcr = new TestCaseResult();
+        if (cwe != -1) {
+            tcr.setCWE(cwe);
+        }
+        tcr.setCategory(category);
+        tcr.setEvidence(category);
+        tcr.setConfidence(confidence);
+        tcr.setNumber(testNumber);
+        return tcr;
+    }
 
     private int cweLookup(String orig) {