From 381ef8958a3d23447e7d4ab46bfc4a8db0dc0f4e Mon Sep 17 00:00:00 2001 From: Sascha Knoop Date: Mon, 26 Aug 2024 18:06:37 +0200 Subject: [PATCH] refactor AppScanReader to use provided CWE (#93) --- .../parsers/HCLAppScanStandardReader.java | 296 +++++------------- .../score/parsers/MendReader.java | 2 +- .../score/parsers/Rapid7Reader.java | 2 +- .../benchmarkutils/score/parsers/Reader.java | 10 +- .../score/parsers/sarif/SarifReader.java | 2 +- .../score/parsers/ReaderTest.java | 12 +- ...hmark_HCLAppScanStandardReader-v10.0.6.xml | 17 - 7 files changed, 90 insertions(+), 251 deletions(-) diff --git a/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/HCLAppScanStandardReader.java b/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/HCLAppScanStandardReader.java index dc608cea..bd2e7c0a 100644 --- a/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/HCLAppScanStandardReader.java +++ b/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 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 variants = getNamedChildren("item", allIssueVariants); - - List 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 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 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 variantLookup( - String issueType, String itemID, String startingUrl, List variants) { - List testCaseElementsFromVariants = new ArrayList<>(); + setTime(root, tr); + setVersion(root, tr); - // System.out.println("Variant Lookup Item ID: " + itemID); + List 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 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; } } diff --git a/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/MendReader.java b/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/MendReader.java index 37a25bd9..e049a075 100644 --- a/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/MendReader.java +++ b/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(); diff --git a/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/Rapid7Reader.java b/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/Rapid7Reader.java index ec376327..336d6dce 100644 --- a/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/Rapid7Reader.java +++ b/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(); diff --git a/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/Reader.java b/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/Reader.java index 421330b6..348e929d 100644 --- a/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/Reader.java +++ b/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 ""; } diff --git a/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/sarif/SarifReader.java b/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/sarif/SarifReader.java index 1f8dfb35..b984a0c8 100644 --- a/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/sarif/SarifReader.java +++ b/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 mappings) { TestCaseResult tcr = new TestCaseResult(); - String className = extractFilename(resultUri(result)); + String className = extractFilenameWithoutEnding(resultUri(result)); if (!className.startsWith(BenchmarkScore.TESTCASENAME)) { return null; diff --git a/plugin/src/test/java/org/owasp/benchmarkutils/score/parsers/ReaderTest.java b/plugin/src/test/java/org/owasp/benchmarkutils/score/parsers/ReaderTest.java index fa2ae2ea..1a6fd05a 100644 --- a/plugin/src/test/java/org/owasp/benchmarkutils/score/parsers/ReaderTest.java +++ b/plugin/src/test/java/org/owasp/benchmarkutils/score/parsers/ReaderTest.java @@ -69,14 +69,14 @@ public void returnsInvalidNumberForPathWithoutTestfile() { "c:\\somepath\\BenchmarkTest00042|BenchmarkTest00042", "c:/somepath/BenchmarkTest00042|BenchmarkTest00042", "/somepath/BenchmarkTest00042|BenchmarkTest00042", - "http://somewhere/BenchmarkTest00042.html|BenchmarkTest00042.html", - "http://somewhere/BenchmarkTest00042.html?foo=bar|BenchmarkTest00042.html", - "https://somewhere:8443/BenchmarkTest00042.html|BenchmarkTest00042.html", + "http://somewhere/BenchmarkTest00042.html|BenchmarkTest00042", + "http://somewhere/BenchmarkTest00042.html?foo=bar|BenchmarkTest00042", + "https://somewhere:8443/BenchmarkTest00042.html|BenchmarkTest00042", "/something/else|else", - "/something/else.html|else.html" + "/something/else.html|else" }, delimiter = '|') - public void extractsFilenameFromPath(String input, String expected) { - assertEquals(expected, Reader.extractFilename(input)); + public void extractsFilenameWithoutEndingFromPath(String input, String expected) { + assertEquals(expected, Reader.extractFilenameWithoutEnding(input)); } } diff --git a/plugin/src/test/resources/testfiles/Benchmark_HCLAppScanStandardReader-v10.0.6.xml b/plugin/src/test/resources/testfiles/Benchmark_HCLAppScanStandardReader-v10.0.6.xml index cb13e778..498c5c75 100644 --- a/plugin/src/test/resources/testfiles/Benchmark_HCLAppScanStandardReader-v10.0.6.xml +++ b/plugin/src/test/resources/testfiles/Benchmark_HCLAppScanStandardReader-v10.0.6.xml @@ -4,26 +4,9 @@ HCL AppScan Standard 10.0.6 - - https://localhost:8443/benchmark - 01:23:45.6789012 - - - https://localhost:8443/benchmark/sqli-00/BenchmarkTest00001 - attSqlInjectionChecks - - - https://localhost:8443/benchmark/sqli-00/BenchmarkTest00002 - attSqlInjectionChecks - - - https://localhost:8443/ - attSameSiteCookie - - 89