diff --git a/dojo/tools/bundler_audit/parser.py b/dojo/tools/bundler_audit/parser.py index f8021588a3d..535cfd4c3e4 100644 --- a/dojo/tools/bundler_audit/parser.py +++ b/dojo/tools/bundler_audit/parser.py @@ -30,7 +30,17 @@ def get_findings(self, filename, test): for warning in warnings: if not warning.startswith("Name"): continue + # Every field below is optional in bundler-audit output. Reset them + # per warning so an absent field cannot inherit the previous + # advisory's value, and so the first warning cannot reference an + # unassigned local. + gem_name = None + gem_version = None advisory_id = None + advisory_title = None + advisory_url = None + advisory_solution = None + sev = None gem_report_fields = warning.split("\n") for field in gem_report_fields: if field.startswith("Name"): @@ -53,25 +63,32 @@ def get_findings(self, filename, test): elif field.startswith("Solution"): advisory_solution = field.replace("Solution: ", "") - title = ( - "Gem " - + gem_name - + ": " - + advisory_title - + " [" - + advisory_id - + "]" - ) + # bundler-audit reports an unrated advisory as "Unknown"; treat an + # absent Criticality line the same way. + if sev is None: + sev = "Medium" + + title = "Gem " + gem_name + if advisory_title: + title += ": " + advisory_title + if advisory_id: + title += " [" + advisory_id + "]" findingdetail = ( "Gem **" + gem_name + "** has known security issues:\n" ) findingdetail += "**Name**: " + gem_name + "\n" - findingdetail += "**Version**: " + gem_version + "\n" - findingdetail += "**Advisory**: " + advisory_id + "\n" + if gem_version: + findingdetail += "**Version**: " + gem_version + "\n" + if advisory_id: + findingdetail += "**Advisory**: " + advisory_id + "\n" mitigation = advisory_solution references = advisory_url fingerprint = ( - "bundler-audit" + gem_name + gem_version + advisory_id + sev + "bundler-audit" + + gem_name + + (gem_version or "") + + (advisory_id or "") + + sev ) dupe_key = hashlib.md5(fingerprint.encode("utf-8"), usedforsecurity=False).hexdigest() if dupe_key in dupes: diff --git a/unittests/scans/bundler_audit/incomplete_first.txt b/unittests/scans/bundler_audit/incomplete_first.txt new file mode 100644 index 00000000000..b032a5e742d --- /dev/null +++ b/unittests/scans/bundler_audit/incomplete_first.txt @@ -0,0 +1,14 @@ +Name: sprockets +Version: 2.2.3 +CVE: CVE-2018-3760 +URL: https://groups.google.com/forum/#!topic/ruby-security-ann/2S9Pwz2i16k + +Name: rack +Version: 1.6.13 +CVE: CVE-2020-8161 +Criticality: High +URL: https://groups.google.com/forum/#!topic/ruby-security-ann/T4ZIsfRf2eA +Title: Directory traversal in Rack::Directory app bundled with Rack +Solution: upgrade to ~> 2.1.3, >= 2.2.0 + +Vulnerabilities found! diff --git a/unittests/scans/bundler_audit/no_advisory_id.txt b/unittests/scans/bundler_audit/no_advisory_id.txt new file mode 100644 index 00000000000..88938aaf4fc --- /dev/null +++ b/unittests/scans/bundler_audit/no_advisory_id.txt @@ -0,0 +1,8 @@ +Name: nokogiri +Version: 1.15.2 +Criticality: Low +URL: https://github.com/sparklemotion/nokogiri/security/advisories +Title: Advisory reported without an assigned identifier +Solution: upgrade to >= 1.16.0 + +Vulnerabilities found! diff --git a/unittests/scans/bundler_audit/partial_fields.txt b/unittests/scans/bundler_audit/partial_fields.txt new file mode 100644 index 00000000000..ceef6159909 --- /dev/null +++ b/unittests/scans/bundler_audit/partial_fields.txt @@ -0,0 +1,14 @@ +Name: rack +Version: 1.6.13 +CVE: CVE-2020-8161 +Criticality: High +URL: https://groups.google.com/forum/#!topic/ruby-security-ann/T4ZIsfRf2eA +Title: Directory traversal in Rack::Directory app bundled with Rack +Solution: upgrade to ~> 2.1.3, >= 2.2.0 + +Name: sprockets +Version: 2.2.3 +CVE: CVE-2018-3760 +URL: https://groups.google.com/forum/#!topic/ruby-security-ann/2S9Pwz2i16k + +Vulnerabilities found! diff --git a/unittests/tools/test_bundler_audit_parser.py b/unittests/tools/test_bundler_audit_parser.py index e7517c56f22..f6c4fcef2c7 100644 --- a/unittests/tools/test_bundler_audit_parser.py +++ b/unittests/tools/test_bundler_audit_parser.py @@ -64,3 +64,51 @@ def test_get_findings_version9(self): self.assertEqual("GHSA-xc9x-jj77-9p9j", finding.unsaved_vulnerability_ids[0]) self.assertEqual("nokogiri", finding.component_name) self.assertEqual("1.15.2", finding.component_version) + + def test_get_findings_missing_optional_fields(self): + """A warning missing optional fields must not inherit the previous warning's values.""" + with (get_unit_tests_scans_path("bundler_audit") / "partial_fields.txt").open(encoding="utf-8") as testfile: + parser = BundlerAuditParser() + findings = parser.get_findings(testfile, Test()) + self.assertEqual(2, len(findings)) + with self.subTest(i=0): + finding = findings[0] + self.assertEqual("Gem rack: Directory traversal in Rack::Directory app bundled with Rack [CVE-2020-8161]", finding.title) + self.assertEqual("High", finding.severity) + self.assertEqual("upgrade to ~> 2.1.3, >= 2.2.0", finding.mitigation) + with self.subTest(i=1): + finding = findings[1] + self.assertEqual("Gem sprockets [CVE-2018-3760]", finding.title) + self.assertEqual("Medium", finding.severity) + self.assertIsNone(finding.mitigation) + self.assertEqual("sprockets", finding.component_name) + self.assertEqual("2.2.3", finding.component_version) + + def test_get_findings_without_advisory_id(self): + """A warning carrying no Advisory/CVE/GHSA line must parse instead of raising.""" + with (get_unit_tests_scans_path("bundler_audit") / "no_advisory_id.txt").open(encoding="utf-8") as testfile: + parser = BundlerAuditParser() + findings = parser.get_findings(testfile, Test()) + self.assertEqual(1, len(findings)) + finding = findings[0] + self.assertEqual("Gem nokogiri: Advisory reported without an assigned identifier", finding.title) + self.assertEqual("Low", finding.severity) + self.assertEqual("nokogiri", finding.component_name) + self.assertEqual("1.15.2", finding.component_version) + self.assertIsNone(finding.unsaved_vulnerability_ids) + + def test_get_findings_incomplete_warning_first(self): + """An incomplete warning with no preceding warning to inherit from must parse.""" + with (get_unit_tests_scans_path("bundler_audit") / "incomplete_first.txt").open(encoding="utf-8") as testfile: + parser = BundlerAuditParser() + findings = parser.get_findings(testfile, Test()) + self.assertEqual(2, len(findings)) + with self.subTest(i=0): + finding = findings[0] + self.assertEqual("Gem sprockets [CVE-2018-3760]", finding.title) + self.assertEqual("Medium", finding.severity) + self.assertIsNone(finding.mitigation) + with self.subTest(i=1): + finding = findings[1] + self.assertEqual("Gem rack: Directory traversal in Rack::Directory app bundled with Rack [CVE-2020-8161]", finding.title) + self.assertEqual("High", finding.severity)