From 86aeeffbd5747f672f72422b54f384bcd6e34a18 Mon Sep 17 00:00:00 2001 From: dogboat Date: Mon, 16 Sep 2024 13:52:06 -0400 Subject: [PATCH] appcheck-severity-determination-fix Use v4, v3, v2 cvss vectors for severity (#10918) * appcheck-severity-determination-fix Use v4, v3, v2 cvss vectors for determining severity in that order; update some type hints * appcheck-severity-determination-fix process cvss-base_score-related variables for severity determination first, then fall back to vectors, then default to "Info" * appcheck-severity-determination-fix fix typo --- .../engines/appcheck.py | 2 +- .../engines/base.py | 93 +++++++++++++++---- ...heck_web_application_scanner_many_vul.json | 2 +- ...appcheck_web_application_scanner_parser.py | 35 ++++++- 4 files changed, 107 insertions(+), 25 deletions(-) diff --git a/dojo/tools/appcheck_web_application_scanner/engines/appcheck.py b/dojo/tools/appcheck_web_application_scanner/engines/appcheck.py index ffcfa4b5632..ba29a780bc7 100644 --- a/dojo/tools/appcheck_web_application_scanner/engines/appcheck.py +++ b/dojo/tools/appcheck_web_application_scanner/engines/appcheck.py @@ -27,7 +27,7 @@ def extract_request_response(self, finding: Finding, value: dict[str, [str]]) -> value.pop("Messages") finding.unsaved_request, finding.unsaved_response = (d.strip() for d in rr_details[0]) - def parse_details(self, finding: Finding, value: dict[str, Union[str, dict[str, [str]]]]) -> None: + def parse_details(self, finding: Finding, value: dict[str, Union[str, dict[str, list[str]]]]) -> None: self.extract_request_response(finding, value) # super's version adds everything else to the description field return super().parse_details(finding, value) diff --git a/dojo/tools/appcheck_web_application_scanner/engines/base.py b/dojo/tools/appcheck_web_application_scanner/engines/base.py index 2b2f1cc1890..f45fd506698 100644 --- a/dojo/tools/appcheck_web_application_scanner/engines/base.py +++ b/dojo/tools/appcheck_web_application_scanner/engines/base.py @@ -5,6 +5,7 @@ import cvss.parser import dateutil.parser from cpe import CPE +from cvss.exceptions import CVSSError from django.core.exceptions import ImproperlyConfigured from dojo.models import Endpoint, Finding @@ -41,6 +42,35 @@ def escape_if_needed(x): return "".join([escape_if_needed(c) for c in s]) +def cvss_score_to_severity(score: float, version: int) -> str: + """ + Maps a CVSS score with a given version to a severity level. + Mapping from https://nvd.nist.gov/vuln-metrics/cvss (modified slightly to have "Info" in range [0.0, 0.1) for CVSS + v3/v4) + """ + cvss_score = float(score) + if version == 2: + if cvss_score >= 7.0: + severity = "High" + elif cvss_score >= 4.0: + severity = "Medium" + else: + severity = "Low" + else: + if cvss_score >= 9.0: + severity = "Critical" + elif cvss_score >= 7.0: + severity = "High" + elif cvss_score >= 4.0: + severity = "Medium" + elif cvss_score >= 0.1: + severity = "Low" + else: + severity = "Info" + + return severity + + ####### # Field parsing helper classes ####### @@ -122,7 +152,6 @@ class BaseEngineParser: * status -> active/false_p/risk_accepted (depending on value) * cves -> unsaved_vulnerability_ids (vulnerability_ids) * cpe -> component name/version - * cvss_vector -> severity (determined using CVSS package) * notes -> appended to Finding description * details -> appended to Finding description @@ -143,7 +172,6 @@ class BaseEngineParser: "status": Method("parse_status"), "cves": Method("parse_cves"), "cpe": Method("parse_components"), - "cvss_vector": Method("parse_severity"), # These should be listed after the 'description' entry; they append to it "notes": Method("parse_notes"), "details": Method("parse_details")} @@ -176,7 +204,7 @@ def parse_initial_date(self, finding: Finding, value: str) -> None: def is_cve(self, c: str) -> bool: return bool(c and isinstance(c, str) and self.CVE_PATTERN.fullmatch(c)) - def parse_cves(self, finding: Finding, value: [str]) -> None: + def parse_cves(self, finding: Finding, value: list[str]) -> None: finding.unsaved_vulnerability_ids = [c.upper() for c in value if self.is_cve(c)] ##### @@ -192,19 +220,6 @@ def parse_status(self, finding: Finding, value: str) -> None: elif value == "acceptable_risk": finding.risk_accepted = True - ##### - # For severity (extracted from cvss vector) - ##### - def get_severity(self, value: str) -> Optional[str]: - if cvss_obj := cvss.parser.parse_cvss_from_text(value): - if (severity := cvss_obj[0].severities()[0].title()) in Finding.SEVERITIES: - return severity - return None - - def parse_severity(self, finding: Finding, value: str) -> None: - if severity := self.get_severity(value): - finding.severity = severity - ##### # For parsing component data ##### @@ -217,7 +232,7 @@ def parse_cpe(self, cpe_str: str) -> (Optional[str], Optional[str]): (cpe_obj.get_version() and cpe_obj.get_version()[0]) or None, ) - def parse_components(self, finding: Finding, value: [str]) -> None: + def parse_components(self, finding: Finding, value: list[str]) -> None: # Only use the first entry finding.component_name, finding.component_version = self.parse_cpe(value[0]) @@ -236,12 +251,12 @@ def append_description(self, finding: Finding, addendum: dict[str, str]) -> None def parse_notes(self, finding: Finding, value: str) -> None: self.append_description(finding, {"Notes": value}) - def extract_details(self, value: Union[str, dict[str, Union[str, dict[str, [str]]]]]) -> dict[str, str]: + def extract_details(self, value: Union[str, dict[str, Union[str, dict[str, list[str]]]]]) -> dict[str, str]: if isinstance(value, dict): return {k: v for k, v in value.items() if k != "_meta"} return {"Details": str(value)} - def parse_details(self, finding: Finding, value: dict[str, Union[str, dict[str, [str]]]]) -> None: + def parse_details(self, finding: Finding, value: dict[str, Union[str, dict[str, list[str]]]]) -> None: self.append_description(finding, self.extract_details(value)) ##### @@ -282,6 +297,44 @@ def set_endpoints(self, finding: Finding, item: Any) -> None: endpoints = self.parse_endpoints(item) finding.unsaved_endpoints.extend(endpoints) + ##### + # For severity (extracted from various cvss vectors) + ##### + def parse_cvss_vector(self, value: str) -> Optional[str]: + # CVSS4 vectors don't parse with the handy-danty parse method :( + try: + if (severity := cvss.CVSS4(value).severity) in Finding.SEVERITIES: + return severity + except CVSSError: + pass + + if cvss_obj := cvss.parser.parse_cvss_from_text(value): + if (severity := cvss_obj[0].severities()[0].title()) in Finding.SEVERITIES: + return severity + return None + + def set_severity(self, finding: Finding, item: Any) -> None: + for base_score_entry, cvss_version in [ + ("cvss_v4_base_score", 4), + ("cvss_v3_base_score", 3), + ("cvss_base_score", 2), + ]: + if base_score := item.get(base_score_entry): + finding.severity = cvss_score_to_severity(base_score, cvss_version) + return + + for vector_type in ["cvss_v4_vector", "cvss_v3_vector", "cvss_vector"]: + if vector := item.get(vector_type): + if severity := self.parse_cvss_vector(vector): + finding.severity = severity + return + + finding.severity = "Info" + + def process_whole_item(self, finding: Finding, item: Any) -> None: + self.set_severity(finding, item) + self.set_endpoints(finding, item) + # Returns the complete field processing map: common fields plus any engine-specific def get_engine_fields(self) -> dict[str, FieldType]: return { @@ -302,7 +355,7 @@ def parse_finding(self, item: dict[str, Any]) -> Tuple[Finding, Tuple]: # Check first whether the field even exists on this item entry; if not, skip it if value := item.get(field): field_handler(self, finding, value) - self.set_endpoints(finding, item) + self.process_whole_item(finding, item) # Make a note of what scanning engine was used for this Finding self.append_description(finding, {"Scanning Engine": self.SCANNING_ENGINE}) return finding, self.get_finding_key(finding) diff --git a/unittests/scans/appcheck_web_application_scanner/appcheck_web_application_scanner_many_vul.json b/unittests/scans/appcheck_web_application_scanner/appcheck_web_application_scanner_many_vul.json index ee12493a840..052de390779 100644 --- a/unittests/scans/appcheck_web_application_scanner/appcheck_web_application_scanner_many_vul.json +++ b/unittests/scans/appcheck_web_application_scanner/appcheck_web_application_scanner_many_vul.json @@ -514,7 +514,7 @@ "cvss_score": 0.0, "type": "WEB_APP", "web_app": "https://example.x73zjffz.com", - "cvss_v4_vector": "CVSS:4.0/AV:N/AC:L/AT:N/PR:N/UI:N/VC:N/VI:N/VA:N/SC:N/SI:N/SA:N", + "cvss_v4_vector": "CVSS:4.0/AV:L/AC:H/AT:P/PR:L/UI:A/VC:N/VI:H/VA:N/SC:N/SI:N/SA:N", "mss_confirmed": false, "category": "web_app", "description": "[[markup]]This is simply a report of HTTP request methods supported by the web application.", diff --git a/unittests/tools/test_appcheck_web_application_scanner_parser.py b/unittests/tools/test_appcheck_web_application_scanner_parser.py index 8928f89abd6..ebe2186a24f 100644 --- a/unittests/tools/test_appcheck_web_application_scanner_parser.py +++ b/unittests/tools/test_appcheck_web_application_scanner_parser.py @@ -4,6 +4,7 @@ from dojo.tools.appcheck_web_application_scanner.engines.appcheck import AppCheckScanningEngineParser from dojo.tools.appcheck_web_application_scanner.engines.base import ( BaseEngineParser, + cvss_score_to_severity, escape_non_printable, strip_markup, ) @@ -96,7 +97,7 @@ def test_appcheck_web_application_scanner_parser_with_many_vuln_has_many_finding self.assertEqual("a25dae3aff97a06b6923b5fc9cc32826e1fd87ab", finding.unique_id_from_tool) self.assertEqual("Apache Tomcat < v9.0.0.M10 - External Control of Assumed-Immutable Web Parameter in JSP Servlet (CVE-2016-6796)", finding.title) self.assertEqual("2024-06-26", finding.date) - self.assertEqual("Medium", finding.severity) + self.assertEqual("High", finding.severity) self.assertEqual(True, finding.active) self.assertEqual("GET Request", finding.unsaved_request) self.assertEqual("Response", finding.unsaved_response) @@ -121,7 +122,7 @@ def test_appcheck_web_application_scanner_parser_with_many_vuln_has_many_finding self.assertEqual("02769aa244c456f0aad810354748faaa70d089c1129dc9c5", finding.unique_id_from_tool) self.assertEqual("Permitted HTTP Methods", finding.title) self.assertEqual("2024-06-27", finding.date) - self.assertEqual("Low", finding.severity) + self.assertEqual("Medium", finding.severity) self.assertEqual(True, finding.active) self.assertIsNone(finding.unsaved_request) self.assertIsNone(finding.unsaved_response) @@ -334,8 +335,15 @@ def test_appcheck_web_application_scanner_parser_base_engine_parser(self): # Invalid cvss vectors ("", None), ("AV:N/AC:H", None), + ("CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:H/A:N", "High"), + ("CVSS:3.0/AV:L/AC:H/PR:H/UI:R/S:U/C:N/I:N/A:N", None), + ("CVSS:4.0/AV:N/AC:L/AT:N/PR:N/UI:N/VC:H/VI:H/VA:N/SC:L/SI:H/SA:H", "Critical"), + ("CVSS:4.0/AV:L/AC:H/AT:P/PR:L/UI:A/VC:N/VI:H/VA:N/SC:N/SI:N/SA:N", "Medium"), + ("CVSS:4.0/AV:L/AC:H/AT:P/PR:L/UI:A/VC:H/VI:H/VA:H/SC:H/SI:N/SA:H", "High"), + ("CVSS:4.0/AV:L/AC:H/AT:P/PR:L/UI:A/VC:N/VI:N/VA:N/SC:H/SI:N/SA:H", "Low"), + ("CVSS:4.0/AV:L/AC:L/AT:N/PR:L/UI:A/VC:N/VI:N/VA:N/SC:N/SI:N/SA:N", None), ]: - self.assertEqual(severity, engine.get_severity(cvss_vector)) + self.assertEqual(severity, engine.parse_cvss_vector(cvss_vector)) # Test component parsing f = Finding() @@ -560,3 +568,24 @@ def test_appcheck_web_application_scanner_parser_non_printable_escape(self): ), ]: self.assertEqual(expected, escape_non_printable(test_string)) + + def test_appcheck_web_application_scanner_parser_cvss_score_mapping(self): + for cvss_score, version, expected in [ + # CVSSv2 + (0.0, 2, "Low"), (0.09, 2, "Low"), (0.1, 2, "Low"), (3.9, 2, "Low"), + (4.0, 2, "Medium"), (5.5, 2, "Medium"), (6.9, 2, "Medium"), + (7.0, 2, "High"), (8.3, 2, "High"), (10.0, 2, "High"), + # CVSSv3 + (0.0, 3, "Info"), (0.09, 3, "Info"), + (0.1, 3, "Low"), (1.2, 3, "Low"), (3.9, 3, "Low"), + (4.0, 3, "Medium"), (5.4, 3, "Medium"), (6.9, 3, "Medium"), + (7.0, 3, "High"), (8.3, 3, "High"), (8.9, 3, "High"), + (9.0, 3, "Critical"), (9.7, 3, "Critical"), (10.0, 3, "Critical"), + # CVSSv4 + (0.0, 4, "Info"), (0.09, 4, "Info"), + (0.1, 4, "Low"), (1.2, 4, "Low"), (3.9, 4, "Low"), + (4.0, 4, "Medium"), (5.4, 4, "Medium"), (6.9, 4, "Medium"), + (7.0, 4, "High"), (8.3, 4, "High"), (8.9, 4, "High"), + (9.0, 4, "Critical"), (9.7, 4, "Critical"), (10.0, 4, "Critical"), + ]: + self.assertEqual(expected, cvss_score_to_severity(cvss_score, version))