[dependency_metadata] Allow descriptions for CVEs

This adds a new way to report CVEs that includes an accompanying
description. It also adds a new validation check that ensures that the
CVE description is present for every entry listed in the 'Mitigated:'
field.

Bug: b/392026683
Change-Id: Ie55595970b49d705ac532f1f8c41ff47d959f56c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6211644
Auto-Submit: Jordan Brown <rop@google.com>
Reviewed-by: Jiewei Qian <qjw@chromium.org>
Commit-Queue: Jiewei Qian <qjw@chromium.org>

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]]:
+    def mitigations(self) -> Dict[str, str]:
-        return self._return_as_property(known_fields.MITIGATED)
+        """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]:

metadata/fields/custom/mitigated.py

@@ -19,9 +19,10 @@ _VULN_PREFIXES = [
     "DSA",  # Debian Security Advisory.
 ]
 
-_PREFIX_PATTERN = "|".join(_VULN_PREFIXES)
+_PATTERN_PREFIX = "|".join(_VULN_PREFIXES)
-_VULN_ID_PATTERN = re.compile(
+PATTERN_VULN_ID = re.compile(
-    rf"^({_PREFIX_PATTERN})-[a-zA-Z0-9]{{4}}-[a-zA-Z0-9:-]+$")
+    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)

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),
+    "^({}){}".format(
-    re.IGNORECASE)
+        "|".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]:

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.

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()

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()