From 94978a2b3be9bf2312114aa0f99c2596d1a77213 Mon Sep 17 00:00:00 2001 From: mhuzaifa3 Date: Tue, 18 Aug 2026 17:51:05 -0500 Subject: [PATCH] fix(bundler-audit): reset advisory fields so warnings cannot inherit stale values get_findings resets advisory_id at the top of each warning block but leaves gem_name, gem_version, advisory_title, advisory_url, advisory_solution and sev bound from the previous iteration. Only Name is guaranteed in bundler-audit output, so a warning that omits any other field is rendered against whatever the preceding warning happened to set. Three consequences, all observable: warning with no Advisory/CVE/GHSA line TypeError: can only concatenate str (not "NoneType") to str incomplete warning with nothing before it UnboundLocalError: cannot access local variable 'advisory_title' incomplete warning after a complete one Gem sprockets: Directory traversal in Rack::Directory [CVE-2018-3760] The third is the damaging one. A sprockets advisory that omits Criticality, Title and Solution takes rack's title, rack's High severity and rack's remediation text, so the finding tells the user to apply rack's fix to sprockets. Resetting every field per warning, and building the title, description and fingerprint from the fields actually present, makes a warning a function of its own text. A missing Criticality line maps to Medium, matching the existing handling of bundler-audit's "Unknown" criticality. Identity is not affected: both existing fixtures parse to byte-identical findings, dedupe fingerprint included, so no recorded sample changes and existing findings do not churn on upgrade. --- dojo/tools/bundler_audit/parser.py | 41 +++++++++++----- .../scans/bundler_audit/incomplete_first.txt | 14 ++++++ .../scans/bundler_audit/no_advisory_id.txt | 8 ++++ .../scans/bundler_audit/partial_fields.txt | 14 ++++++ unittests/tools/test_bundler_audit_parser.py | 48 +++++++++++++++++++ 5 files changed, 113 insertions(+), 12 deletions(-) create mode 100644 unittests/scans/bundler_audit/incomplete_first.txt create mode 100644 unittests/scans/bundler_audit/no_advisory_id.txt create mode 100644 unittests/scans/bundler_audit/partial_fields.txt 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)