fix(bundler-audit): reset advisory fields so warnings cannot inherit stale values

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:

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!

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!

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!

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)