← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] ~enriqueesanchz/launchpad:add-priority-explanation into launchpad:master

 

Enrique Sánchez has proposed merging ~enriqueesanchz/launchpad:add-priority-explanation into launchpad:master.

Commit message:
Add priority_explanation support

Add priority_explanation support for cve import/export scripts
Add priority_explanation field to UCTRecord
Add importance_explanation field to CVE
Add sampledata/CVE-2023-32637 that has priority_explanation

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~enriqueesanchz/launchpad/+git/launchpad/+merge/484565
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~enriqueesanchz/launchpad:add-priority-explanation into launchpad:master.
diff --git a/lib/lp/bugs/scripts/tests/sampledata/CVE-2023-32637 b/lib/lp/bugs/scripts/tests/sampledata/CVE-2023-32637
new file mode 100644
index 0000000..161dc4d
--- /dev/null
+++ b/lib/lp/bugs/scripts/tests/sampledata/CVE-2023-32637
@@ -0,0 +1,29 @@
+Candidate: CVE-2023-32637
+PublicDate: 2023-07-25 06:15:00 UTC
+References:
+ https://jvn.jp/en/jp/JVN35897618/
+ https://jbrowse.org/jb2/
+ http://gmod.org/wiki/GBrowse
+ https://www.cve.org/CVERecord?id=CVE-2023-32637
+Description:
+ GBrowse accepts files with any formats uploaded and places them in the area
+ accessible through unauthenticated web requests. Therefore, anyone who can
+ upload files through the product may execute arbitrary code on the server.
+Ubuntu-Description:
+Notes:
+ ccdm94> this has likely been fixed in all 2.x versions.
+Mitigation:
+Bugs:
+Priority: high
+ This has a high priority because it is a vulnerability that allows a remote
+ attacker to execute code in a machine, and it looks to be easily exploitable
+ given that it involves regular functionalities provided by the application.
+Discovered-by:
+Assigned-to:
+CVSS:
+ nvd: CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H [9.8 CRITICAL]
+
+Patches_gbrowse:
+upstream_gbrowse: released (2.56+dfsg-1)
+trusty_gbrowse: ignored (end of standard support)
+xenial_gbrowse: ignored (end of standard support)
diff --git a/lib/lp/bugs/scripts/tests/test_uct.py b/lib/lp/bugs/scripts/tests/test_uct.py
index f029f28..51da3db 100644
--- a/lib/lp/bugs/scripts/tests/test_uct.py
+++ b/lib/lp/bugs/scripts/tests/test_uct.py
@@ -170,6 +170,94 @@ class TestUCTRecord(TestCase):
         )
         self.assertEqual(load_from.read_text(), saved_to_path.read_text())
 
+    def test_load_save_with_priority_explanation(self):
+        load_from = Path(__file__).parent / "sampledata" / "CVE-2023-32637"
+        uct_record = UCTRecord.load(load_from)
+        self.assertDictEqual(
+            UCTRecord(
+                parent_dir="sampledata",
+                assigned_to="",
+                bugs=[""],
+                cvss=[
+                    CVSS(
+                        authority="nvd",
+                        vector_string=(
+                            "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H "
+                            "[9.8 CRITICAL]"
+                        ),
+                    ),
+                ],
+                candidate="CVE-2023-32637",
+                crd=None,
+                public_date_at_USN=None,
+                public_date=datetime(2023, 7, 25, 6, 15, tzinfo=timezone.utc),
+                description=(
+                    "GBrowse accepts files with any formats uploaded and "
+                    "places them in the area\naccessible through "
+                    "unauthenticated web requests. Therefore, anyone who can\n"
+                    "upload files through the product may execute arbitrary "
+                    "code on the server."
+                ),
+                discovered_by="",
+                mitigation="",
+                notes=(
+                    "ccdm94> this has likely been fixed in all 2.x "
+                    "versions."
+                ),
+                priority=UCTRecord.Priority.HIGH,
+                priority_explanation=(
+                    "This has a high priority because it is a vulnerability "
+                    "that allows a remote\nattacker to execute code in a "
+                    "machine, and it looks to be easily exploitable\ngiven "
+                    "that it involves regular functionalities provided by the "
+                    "application."
+                ),
+                references=[
+                    "https://jvn.jp/en/jp/JVN35897618/";,
+                    "https://jbrowse.org/jb2/";,
+                    "http://gmod.org/wiki/GBrowse";,
+                    "https://www.cve.org/CVERecord?id=CVE-2023-32637";,
+                ],
+                ubuntu_description="",
+                packages=[
+                    UCTRecord.Package(
+                        name="gbrowse",
+                        statuses=[
+                            UCTRecord.SeriesPackageStatus(
+                                series="upstream",
+                                status=UCTRecord.PackageStatus.RELEASED,
+                                reason="2.56+dfsg-1",
+                                priority=None,
+                            ),
+                            UCTRecord.SeriesPackageStatus(
+                                series="trusty",
+                                status=UCTRecord.PackageStatus.IGNORED,
+                                reason="end of standard support",
+                                priority=None,
+                            ),
+                            UCTRecord.SeriesPackageStatus(
+                                series="xenial",
+                                status=UCTRecord.PackageStatus.IGNORED,
+                                reason="end of standard support",
+                                priority=None,
+                            ),
+                        ],
+                        priority=None,
+                        tags=set(),
+                        patches=[],
+                    ),
+                ],
+            ).__dict__,
+            uct_record.__dict__,
+        )
+
+        output_dir = Path(self.makeTemporaryDirectory())
+        saved_to_path = uct_record.save(output_dir)
+        self.assertEqual(
+            output_dir / "sampledata" / "CVE-2023-32637", saved_to_path
+        )
+        self.assertEqual(load_from.read_text(), saved_to_path.read_text())
+
 
 class TestCVE(TestCaseWithFactory):
     layer = ZopelessDatabaseLayer
@@ -231,6 +319,7 @@ class TestCVE(TestCaseWithFactory):
             mitigation="mitigation",
             notes="author> text",
             priority=UCTRecord.Priority.CRITICAL,
+            priority_explanation="sample priority_explanation",
             references=["https://ubuntu.com/security/notices/USN-5368-1";],
             ubuntu_description="ubuntu-description",
             packages=[
@@ -416,6 +505,7 @@ class TestCVE(TestCaseWithFactory):
                 ),
             ],
             importance=BugTaskImportance.CRITICAL,
+            importance_explanation="sample priority_explanation",
             status=VulnerabilityStatus.ACTIVE,
             assignee=assignee,
             discovered_by="tr3e wang",
diff --git a/lib/lp/bugs/scripts/uct/models.py b/lib/lp/bugs/scripts/uct/models.py
index 91510e9..d5815b6 100644
--- a/lib/lp/bugs/scripts/uct/models.py
+++ b/lib/lp/bugs/scripts/uct/models.py
@@ -118,6 +118,7 @@ class UCTRecord:
         references: List[str],
         ubuntu_description: str,
         packages: List[Package],
+        priority_explanation: str = "",
     ):
         self.parent_dir = parent_dir
         self.assigned_to = assigned_to
@@ -132,6 +133,7 @@ class UCTRecord:
         self.mitigation = mitigation
         self.notes = notes
         self.priority = priority
+        self.priority_explanation = priority_explanation
         self.references = references
         self.ubuntu_description = ubuntu_description
         self.packages = packages
@@ -233,6 +235,8 @@ class UCTRecord:
                 )
             )
 
+        _priority = cls._pop_cve_property(cve_data, "Priority").split("\n")
+
         entry = UCTRecord(
             parent_dir=cve_path.absolute().parent.name,
             assigned_to=cls._pop_cve_property(cve_data, "Assigned-to"),
@@ -254,7 +258,8 @@ class UCTRecord:
                 cve_data, "Mitigation", required=False
             ),
             notes=cls._format_notes(cls._pop_cve_property(cve_data, "Notes")),
-            priority=cls.Priority(cls._pop_cve_property(cve_data, "Priority")),
+            priority=cls.Priority(_priority[0]),
+            priority_explanation="\n".join(_priority[1:]),
             references=cls._pop_cve_property(cve_data, "References").split(
                 "\n"
             ),
@@ -305,7 +310,11 @@ class UCTRecord:
                 "Mitigation", self.mitigation.split("\n"), output
             )
         self._write_field("Bugs", self.bugs, output)
-        self._write_field("Priority", self.priority.value, output)
+        self._write_field(
+            "Priority",
+            self._format_priority(self.priority, self.priority_explanation),
+            output,
+        )
         self._write_field("Discovered-by", self.discovered_by, output)
         self._write_field("Assigned-to", self.assigned_to, output)
         self._write_field(
@@ -400,6 +409,15 @@ class UCTRecord:
                 lines.append("  " + line)
         return "\n".join(lines)
 
+    @classmethod
+    def _format_priority(cls, priority: Priority, explanation: str) -> str:
+        lines = [priority.value]
+        for line in explanation.split("\n"):
+            if line != "":
+                lines.append(" " + line)
+
+        return "\n".join(lines)
+
 
 class CVE:
     """
@@ -493,6 +511,7 @@ class CVE:
         mitigation: str,
         cvss: List[CVSS],
         patch_urls: Optional[List[PatchURL]] = None,
+        importance_explanation: str = "",
     ):
         self.sequence = sequence
         self.date_made_public = date_made_public
@@ -502,6 +521,7 @@ class CVE:
         self.series_packages = series_packages
         self.upstream_packages = upstream_packages
         self.importance = importance
+        self.importance_explanation = importance_explanation
         self.status = status
         self.assignee = assignee
         self.discovered_by = discovered_by
@@ -647,6 +667,7 @@ class CVE:
             series_packages=series_packages,
             upstream_packages=upstream_packages,
             importance=cls.PRIORITY_MAP[uct_record.priority],
+            importance_explanation=uct_record.priority_explanation,
             status=cls.infer_vulnerability_status(uct_record),
             assignee=assignee,
             discovered_by=uct_record.discovered_by,
@@ -774,6 +795,7 @@ class CVE:
             mitigation=self.mitigation,
             notes=self.notes,
             priority=self.PRIORITY_MAP_REVERSE[self.importance],
+            priority_explanation=self.importance_explanation,
             references=self.references,
             ubuntu_description=self.ubuntu_description,
             packages=list(packages_by_name.values()),
diff --git a/lib/lp/bugs/scripts/uct/uctexport.py b/lib/lp/bugs/scripts/uct/uctexport.py
index 3745874..9b2045a 100644
--- a/lib/lp/bugs/scripts/uct/uctexport.py
+++ b/lib/lp/bugs/scripts/uct/uctexport.py
@@ -231,6 +231,7 @@ class UCTExporter:
             series_packages=series_packages,
             upstream_packages=upstream_packages,
             importance=cve_importance,
+            importance_explanation=vulnerability.importance_explanation,
             status=vulnerability.status,
             assignee=bug_tasks[0].assignee,
             discovered_by=lp_cve.discovered_by or "",
diff --git a/lib/lp/bugs/scripts/uct/uctimport.py b/lib/lp/bugs/scripts/uct/uctimport.py
index 8ee237b..11f62a3 100644
--- a/lib/lp/bugs/scripts/uct/uctimport.py
+++ b/lib/lp/bugs/scripts/uct/uctimport.py
@@ -313,6 +313,7 @@ class UCTImporter:
             distribution=distribution,
             status=cve.status,
             importance=cve.importance,
+            importance_explanation=cve.importance_explanation,
             creator=bug.owner,
             information_type=InformationType.PUBLICSECURITY,
             cve=lp_cve,
@@ -362,6 +363,7 @@ class UCTImporter:
         vulnerability.notes = cve.notes
         vulnerability.mitigation = cve.mitigation
         vulnerability.importance = cve.importance
+        vulnerability.importance_explanation = cve.importance_explanation
         vulnerability.date_made_public = date_made_public
         vulnerability.date_notice_issued = date_notice_issued
         vulnerability.date_coordinated_release = date_coordinated_release