diff --git a/metadata/dependency_metadata.py b/metadata/dependency_metadata.py index 14e964240..8fc3b62e4 100644 --- a/metadata/dependency_metadata.py +++ b/metadata/dependency_metadata.py @@ -19,6 +19,7 @@ sys.path.insert(0, _ROOT_DIR) import metadata.fields.field_types as field_types import metadata.fields.custom.license as license_util import metadata.fields.custom.version as version_util +import metadata.fields.custom.mitigated as mitigated_util import metadata.fields.known as known_fields import metadata.fields.util as util import metadata.validation_result as vr @@ -285,8 +286,42 @@ class DependencyMetadata: self.get_field_line_numbers(known_fields.LICENSE)) results.append(license_result) + # Match values reported in the 'Mitigated:' field with the supplementry + # fields e.g. 'CVE-2024-12345: description'. + mitigated_values = self._return_as_property(known_fields.MITIGATED) + mitigated_ids = set() + if mitigated_values is not None: + mitigated_ids = set(mitigated_values) + # Reported as their own field e.g. 'CVE-2024-12345: description'. + mitigated_entries = set(self._mitigations_from_entries().keys()) + + missing_descriptions = mitigated_ids - mitigated_entries + if missing_descriptions: + results.append( + vr.ValidationWarning( + reason="Missing descriptions for vulnerability IDs", + additional=[ + f"Add descriptions for: {util.quoted(missing_descriptions)}" + ])) + + extra_descriptions = mitigated_entries - mitigated_ids + if extra_descriptions: + results.append( + vr.ValidationWarning( + reason="Found descriptions for unlisted vulnerability IDs", + additional=[ + f"List these IDs in the 'Mitigated:' field: {util.quoted(extra_descriptions)}" + ])) + return results + def _mitigations_from_entries(self) -> Dict[str, str]: + result = {} + for key, value in self._entries: + if mitigated_util.PATTERN_VULN_ID_WITH_ANCHORS.match(key): + result[key] = value.strip() + return result + def _return_as_property(self, field: field_types.MetadataField) -> Any: """Helper function to create a property for DependencyMetadata. @@ -306,8 +341,16 @@ class DependencyMetadata: return self._return_as_property(known_fields.NAME) @property - def mitigated(self) -> Optional[List[str]]: - return self._return_as_property(known_fields.MITIGATED) + def mitigations(self) -> Dict[str, str]: + """Returns mapping of vulnerability IDs to their descriptions.""" + result = self._mitigations_from_entries() + mitigated_values = self._return_as_property(known_fields.MITIGATED) or [] + # Add entries listed in Mitigated field but without a supplement + # mitigation description line. + for id in mitigated_values: + if id not in result: + result[id] = "" + return result @property def short_name(self) -> Optional[str]: diff --git a/metadata/fields/custom/mitigated.py b/metadata/fields/custom/mitigated.py index f44c5cf9d..08c4e93b5 100644 --- a/metadata/fields/custom/mitigated.py +++ b/metadata/fields/custom/mitigated.py @@ -19,9 +19,10 @@ _VULN_PREFIXES = [ "DSA", # Debian Security Advisory. ] -_PREFIX_PATTERN = "|".join(_VULN_PREFIXES) -_VULN_ID_PATTERN = re.compile( - rf"^({_PREFIX_PATTERN})-[a-zA-Z0-9]{{4}}-[a-zA-Z0-9:-]+$") +_PATTERN_PREFIX = "|".join(_VULN_PREFIXES) +PATTERN_VULN_ID = re.compile( + rf"({_PATTERN_PREFIX})-[a-zA-Z0-9]{{4}}-[a-zA-Z0-9:-]+") +PATTERN_VULN_ID_WITH_ANCHORS = re.compile(f"^{PATTERN_VULN_ID.pattern}$") def validate_vuln_ids(vuln_ids: str) -> Tuple[List[str], List[str]]: @@ -46,7 +47,7 @@ def validate_vuln_ids(vuln_ids: str) -> Tuple[List[str], List[str]]: for cve in vuln_ids.split(","): cve_stripped = cve.strip() - if _VULN_ID_PATTERN.match(cve_stripped): + if PATTERN_VULN_ID_WITH_ANCHORS.match(cve_stripped): valid_vuln_ids.append(cve_stripped) else: invalid_vuln_ids.append(cve) diff --git a/metadata/parse.py b/metadata/parse.py index debaa34ce..9f350083a 100644 --- a/metadata/parse.py +++ b/metadata/parse.py @@ -17,6 +17,7 @@ sys.path.insert(0, _ROOT_DIR) import metadata.fields.known as known_fields import metadata.dependency_metadata as dm +import metadata.fields.custom.mitigated # Line used to separate dependencies within the same metadata file. DEPENDENCY_DIVIDER = re.compile(r"^-{20} DEPENDENCY DIVIDER -{20}$") @@ -32,10 +33,13 @@ _PATTERN_FIELD_NAME_HEURISTIC = re.compile(r"^({}(?: {})*){}[\b\s]".format( _DEFAULT_TO_STRUCTURED_TEXT = False # Pattern used to check if a line from a metadata file declares a new -# field. +# field. This includes all valid vulnerability IDs. _PATTERN_KNOWN_FIELD_DECLARATION = re.compile( - "^({}){}".format("|".join(known_fields.ALL_FIELD_NAMES), FIELD_DELIMITER), - re.IGNORECASE) + "^({}){}".format( + "|".join( + list(known_fields.ALL_FIELD_NAMES) + + [metadata.fields.custom.mitigated.PATTERN_VULN_ID.pattern]), + FIELD_DELIMITER), re.IGNORECASE) def parse_content(content: str) -> List[dm.DependencyMetadata]: diff --git a/metadata/tests/data/README.chromium.test.mitigated b/metadata/tests/data/README.chromium.test.mitigated new file mode 100644 index 000000000..e8feefa3c --- /dev/null +++ b/metadata/tests/data/README.chromium.test.mitigated @@ -0,0 +1,21 @@ +Name: Test dependency with mitigated CVEs +Short Name: cve-test +URL: https://www.example.com/metadata +Version: 1.0.12 +Date: 2020-12-03 +License: MIT +License File: LICENSE +Security Critical: yes +Shipped: yes +CPEPrefix: unknown +Mitigated: CVE-2011-4061, CVE-2024-7255 ,CVE-2024-7256 +CVE-2011-4061: This copy of DependencyA only includes rainbows +that spill beautifully over multiple lines and are handled + ~~ Perfectly ~~ +Even: this line with colons that mentions CVE-2000-2000: an unrelated cve. +CVE-2024-7255: This copy of DependencyA only includes unicorns +CVE-2024-7256: This also doesn't apply because of good reasons +Description: A test dependency with mitigated CVE entries. + +Local Modifications: +None. \ No newline at end of file diff --git a/metadata/tests/dependency_metadata_test.py b/metadata/tests/dependency_metadata_test.py index 546d4f1e7..86564dd01 100644 --- a/metadata/tests/dependency_metadata_test.py +++ b/metadata/tests/dependency_metadata_test.py @@ -1,4 +1,3 @@ - #!/usr/bin/env vpython3 # Copyright (c) 2023 The Chromium Authors. All rights reserved. # Use of this source code is governed by a BSD-style license that can be @@ -405,5 +404,55 @@ class DependencyValidationTest(unittest.TestCase): result = dependency.only_open_source_licenses("InvalidLicense, MPL-2.0") self.assertEqual(result, ["MPL-2.0"]) + def test_mitigated_validation(self): + """Tests validation of Mitigated field and corresponding CVE descriptions.""" + dependency = dm.DependencyMetadata() + dependency.add_entry(known_fields.NAME.get_name(), "Test Dependency") + dependency.add_entry(known_fields.URL.get_name(), "http://example.com") + dependency.add_entry(known_fields.VERSION.get_name(), "1.0") + dependency.add_entry(known_fields.LICENSE.get_name(), "MIT") + dependency.add_entry(known_fields.LICENSE_FILE.get_name(), "LICENSE") + dependency.add_entry(known_fields.SECURITY_CRITICAL.get_name(), "yes") + dependency.add_entry(known_fields.SHIPPED.get_name(), "yes") + + # Add description for one CVE and an extra one. + dependency.add_entry("CVE-2024-1234", "Fixed in this version") + dependency.add_entry("CVE-2024-9999", "This shouldn't be here") + + results = dependency.validate( + source_file_dir=os.path.join(_THIS_DIR, "data"), + repo_root_dir=_THIS_DIR, + ) + # Check that a warning is returned when only CVE descriptions are + # present. + self.assertEqual(len(results), 1) + self.assertTrue(isinstance(results[0], vr.ValidationWarning)) + self.assertEqual(results[0].get_reason(), + "Found descriptions for unlisted vulnerability IDs") + self.assertIn("CVE-2024-1234",results[0].get_additional()[0]) + self.assertIn("CVE-2024-9999",results[0].get_additional()[0]) + + # Add Mitigated field with two CVEs. + dependency.add_entry(known_fields.MITIGATED.get_name(), + "CVE-2024-1234, CVE-2024-5678") + + results = dependency.validate( + source_file_dir=os.path.join(_THIS_DIR, "data"), + repo_root_dir=_THIS_DIR, + ) + + # Should get two warnings: + # 1. Missing description for CVE-2024-5678 + # 2. Extra description for CVE-2024-9999 + self.assertEqual(len(results), 2) + self.assertTrue(isinstance(results[0], vr.ValidationWarning)) + self.assertEqual(results[0].get_reason(), + "Missing descriptions for vulnerability IDs") + self.assertIn("CVE-2024-5678",results[0].get_additional()[0]) + self.assertTrue(isinstance(results[1], vr.ValidationWarning)) + self.assertEqual(results[1].get_reason(), + "Found descriptions for unlisted vulnerability IDs") + self.assertIn("CVE-2024-9999",results[1].get_additional()[0]) + if __name__ == "__main__": unittest.main() diff --git a/metadata/tests/parse_test.py b/metadata/tests/parse_test.py index cc8f03ef1..69d833b69 100644 --- a/metadata/tests/parse_test.py +++ b/metadata/tests/parse_test.py @@ -224,5 +224,28 @@ lowlist.py). Licenses not allowlisted: 'Custom license'."""), self.assertEqual(dm.get_field_line_numbers(metadata.fields.known.NAME), [1]) + def test_parse_mitigated(self): + """Check parsing works for mitigated CVE entries.""" + filepath = os.path.join(_THIS_DIR, "data", + "README.chromium.test.mitigated") + content = gclient_utils.FileRead(filepath) + all_metadata = metadata.parse.parse_content(content) + + self.assertEqual(len(all_metadata), 1) + + # Check that the CVEs are properly parsed. + self.assertDictEqual( + all_metadata[0].mitigations, + { + "CVE-2011-4061": + "This copy of DependencyA only includes rainbows\nthat spill beautifully over multiple lines and are handled\n ~~ Perfectly ~~\nEven: this line with colons that mentions CVE-2000-2000: an unrelated cve.", + "CVE-2024-7255": + "This copy of DependencyA only includes unicorns", + "CVE-2024-7256": + "This also doesn't apply because of good reasons" + }, + ) + + if __name__ == "__main__": unittest.main()