diff --git a/dojo/tools/qualys/parser.py b/dojo/tools/qualys/parser.py index 465301310f4..ad1072a7d32 100644 --- a/dojo/tools/qualys/parser.py +++ b/dojo/tools/qualys/parser.py @@ -201,7 +201,7 @@ def split_cvss(value, _temp): _temp["CVSS_value"] = float(value) -def parse_finding(host, tree): +def parse_finding(host, glossary): ret_rows = [] issue_row = {} @@ -265,8 +265,7 @@ def parse_finding(host, tree): # DefectDojo does not support cvssv2 temp["CVSS_vector"] = None - search = f".//GLOSSARY/VULN_DETAILS_LIST/VULN_DETAILS[@id='{gid}']" - vuln_item = tree.find(search) + vuln_item = glossary.get(gid) if vuln_item is not None: finding = Finding() # Vuln name @@ -373,6 +372,21 @@ def qualys_parser(qualys_xml_file): host_list = tree.find("HOST_LIST") finding_list = [] if host_list is not None: + glossary = vuln_details_by_id(tree) for host in host_list: - finding_list += parse_finding(host, tree) + finding_list += parse_finding(host, glossary) return finding_list + + +def vuln_details_by_id(tree): + """ + The report's glossary entries by QID, read once per report. + + Looking a QID up with a search from the document root walks every host before it reaches the + glossary, so doing that for each finding made a parse cost findings x report size. The first + entry for an id wins, as that search returned it. + """ + glossary = {} + for vuln_item in tree.iterfind(".//GLOSSARY/VULN_DETAILS_LIST/VULN_DETAILS"): + glossary.setdefault(vuln_item.get("id"), vuln_item) + return glossary diff --git a/unittests/tools/test_qualys_parser.py b/unittests/tools/test_qualys_parser.py index 060b6b9fcc0..814b6ae12fe 100644 --- a/unittests/tools/test_qualys_parser.py +++ b/unittests/tools/test_qualys_parser.py @@ -1,4 +1,8 @@ import datetime +import io +import re +from unittest import mock +from xml.etree import ElementPath from django.test import override_settings @@ -7,8 +11,63 @@ from unittests.dojo_test_case import DojoTestCase, get_unit_tests_scans_path +def _report_with_hosts(count): + """The sample report with its hosts repeated to ``count`` hosts, each with its own address.""" + sample = (get_unit_tests_scans_path("qualys") / "Qualys_Sample_Report.xml").read_text(encoding="utf-8") + head, rest = sample.split("", 1) + hosts, tail = rest.split("", 1) + blocks = re.findall(r".*?", hosts, flags=re.DOTALL) + repeated = [ + re.sub(r"[^<]*", f"192.0.2.{index + 1}", blocks[index % len(blocks)], count=1) + for index in range(count) + ] + return _named(f"{head}{''.join(repeated)}{tail}") + + +def _named(content): + report = io.BytesIO(content.encode()) + report.name = "report.xml" + return report + + class TestQualysParser(DojoTestCase): + def test_glossary_is_searched_once_per_report_not_once_per_finding(self): + # Every finding used to look its QID up with a search from the document root, which walks + # all the hosts before it reaches the glossary: the parse was findings x document size. + # Every find, findall, findtext and iterfind, on an element or on the whole tree, resolves its + # path through ElementPath.iterfind, so a per-QID lookup from any starting point is counted. + searches = [] + original = ElementPath.iterfind + + def counting(elem, path, namespaces=None): + if "VULN_DETAILS" in path: + searches.append(path) + return original(elem, path, namespaces) + + patcher = mock.patch.object(ElementPath, "iterfind", counting) + patcher.start() + self.addCleanup(patcher.stop) + + small = QualysParser().get_findings(_report_with_hosts(2), Test()) + small_searches = len(searches) + searches.clear() + large = QualysParser().get_findings(_report_with_hosts(14), Test()) + + self.assertGreater(len(large), len(small)) + self.assertEqual(small_searches, len(searches)) + + def test_repeated_glossary_entry_keeps_the_first(self): + sample = (get_unit_tests_scans_path("qualys") / "Qualys_Sample_Report.xml").read_text(encoding="utf-8") + details = re.search(r".*?", sample, flags=re.DOTALL).group(0) + shadow = re.sub(r"<!\[CDATA\[[^\]]*\]\]>", "<![CDATA[Shadowed]]>", details, count=1) + report = _named(sample.replace("", f"{shadow}", 1)) + + titles = {finding.title for finding in QualysParser().get_findings(report, Test())} + + self.assertIn("QID-6 | DNS Host Name", titles) + self.assertNotIn("QID-6 | Shadowed", titles) + @override_settings(USE_FIRST_SEEN=True) def test_parse_file_with_no_vuln_has_no_findings_first_seen(self): self.parse_file_with_no_vuln_has_no_findings()