Or should we rather rename all those patches which violate current code? What is then the meaning of CVE: tag within the patch if it's not used but it's enforced? We should probably remove cve_payload_tag handling from test_cve_tag_format...
Peter > -----Original Message----- > From: [email protected] <openembedded- > [email protected]> On Behalf Of Peter Marko via > lists.openembedded.org > Sent: Friday, January 10, 2025 9:34 > To: [email protected]; [email protected] > Subject: Re: [OE-core] [PATCH v2 1/1] cve-check: Rework patch parsing > > Hello, > > This patch has caused a regression in detecting cve patches. > Master jumped by 32 CVEs after merging this commit. > These are for sure patched but it's not detected anymore. > See https://valkyrie.yocto.io/pub/non-release/patchmetrics/ > (master should be identical to scarthgap/styhead except for kernel CVEs) > > Reports cannot be used anymore for picking CVEs for TODO list. > So this patch should be reverted or fixed. > > Peter > > > -----Original Message----- > > From: [email protected] <openembedded- > > [email protected]> On Behalf Of Colin McAllister via > > lists.openembedded.org > > Sent: Monday, December 30, 2024 20:22 > > To: [email protected] > > Cc: Colin McAllister <[email protected]> > > Subject: [OE-core] [PATCH v2 1/1] cve-check: Rework patch parsing > > > > The cve_check functionality to parse CVE IDs from the patch filename and > > patch contents have been reworked to improve parsing and also utilize > > tests. This ensures that the parsing works as intended. > > > > Additionally, the new patched_cves dict has a few issues I tried to fix > > as well. If multiple patch files exist for a single CVE ID, only the > > last one will show up with the "resource" key. The value for the > > "resource" key has been updated to hold a list and return all patch > > files associated with a given CVE ID. Also, at the end of > > get_patch_cves, CVE_STATUS can overwrite an existing entry in the dict. > > This could cause an issue, for example, if a CVE has been addressed via > > a patch, but a CVE_STATUS line also exists that ignores the given CVE > > ID. A warning has been added if this ever happens. > > > > Signed-off-by: Colin McAllister <[email protected]> > > --- > > > > I noticed that there are some patches, especially in older verisons of > > Yocto, where the "CVE: " tag was used with multiple CVE IDs in different > > formats, like "CVE-YYYY-XXXX & CVE-YYYY-XXXX" or > > "CVE-YYYY-XXXX, CVE-YYYY-XXXX". Currently, only space-delimited CVE > > IDs will be parsed, but documentation doesn't indicate that is the only > > supported format. I figured it'd be nice to update the code to be able > > to support multiple formats, that way this patch could be backported to > > fix those patches. I also wanted to add unit tests to ensure the patch > > parsing behavior is preserved. > > > > I'd also like to update the patch filename parsing to parse multiple CVE > > IDs from the filename, but based on the comments, it seems like there > > was a reason why only the last CVE ID is extracted from the filename. > > I'd be happy to submit a V2 patch or an additional patch to update the > > function if that sounds good for the maintainers. > > > > V2 Changes: > > I mistakenly created this patch without fb3f440 applied. I updated > > get_patched_cves to return a dict instead of a set. > > > > I also improved the docstrings for these new functions to be more > > descriptive and also specify the return types. > > > > I also found a few issues with the dictionary object created by > > get_patched_cves that have now been addressed in this commit. > > > > meta/lib/oe/cve_check.py | 166 ++++++++++++------ > > meta/lib/oeqa/selftest/cases/cve_check.py | 205 > > ++++++++++++++++++++++ > > 2 files changed, 317 insertions(+), 54 deletions(-) > > > > diff --git a/meta/lib/oe/cve_check.py b/meta/lib/oe/cve_check.py > > index 280f9f613d..85a899a880 100644 > > --- a/meta/lib/oe/cve_check.py > > +++ b/meta/lib/oe/cve_check.py > > @@ -5,9 +5,11 @@ > > # > > > > import collections > > -import re > > -import itertools > > import functools > > +import itertools > > +import os.path > > +import re > > +import oe.patch > > > > _Version = collections.namedtuple( > > "_Version", ["release", "patch_l", "pre_l", "pre_v"] > > @@ -71,76 +73,132 @@ def _cmpkey(release, patch_l, pre_l, pre_v): > > return _release, _patch, _pre > > > > > > -def get_patched_cves(d): > > +def parse_cve_from_filename(patch_filename): > > """ > > - Get patches that solve CVEs using the "CVE: " tag. > > + Parses CVE ID from the filename > > + > > + Matches the last "CVE-YYYY-ID" in the file name, also if written > > + in lowercase. Possible to have multiple CVE IDs in a single > > + file name, but only the last one will be detected from the file name. > > + > > + Returns the last CVE ID foudn in the filename. If no CVE ID is found > > + an empty string is returned. > > """ > > + cve_file_name_match = re.compile(r".*(CVE-\d{4}-\d{4,})", > > re.IGNORECASE) > > > > - import re > > - import oe.patch > > + # Check patch file name for CVE ID > > + fname_match = cve_file_name_match.search(patch_filename) > > + return fname_match.group(1).upper() if fname_match else "" > > > > - cve_match = re.compile(r"CVE:( CVE-\d{4}-\d+)+") > > > > - # Matches the last "CVE-YYYY-ID" in the file name, also if written > > - # in lowercase. Possible to have multiple CVE IDs in a single > > - # file name, but only the last one will be detected from the file name. > > - # However, patch files contents addressing multiple CVE IDs are > supported > > - # (cve_match regular expression) > > - cve_file_name_match = re.compile(r".*(CVE-\d{4}-\d+)", > re.IGNORECASE) > > +def parse_cves_from_patch_contents(patch_contents): > > + """ > > + Parses CVE IDs from patch contents > > > > + Matches all CVE IDs contained on a line that starts with "CVE: ". Any > > + delimiter (',', '&', "and", etc.) can be used without any issues. > > Multiple > > + "CVE:" lines can also exist. > > + > > + Returns a set of all CVE IDs found in the patch contents. > > + """ > > + cve_ids = set() > > + cve_match = re.compile(r"CVE-\d{4}-\d{4,}") > > + # Search for one or more "CVE: " lines > > + for line in patch_contents.split("\n"): > > + if not line.startswith("CVE:"): > > + continue > > + cve_ids.update(cve_match.findall(line)) > > + return cve_ids > > + > > + > > +def parse_cves_from_patch_file(patch_file): > > + """ > > + Parses CVE IDs associated with a particular patch file, using both the > > filename > > + and patch contents. > > + > > + Returns a set of all CVE IDs found in the patch filename and contents. > > + """ > > + cve_ids = set() > > + filename_cve = parse_cve_from_filename(patch_file) > > + if filename_cve: > > + bb.debug(2, "Found %s from patch file name %s" % (filename_cve, > > patch_file)) > > + cve_ids.add(parse_cve_from_filename(patch_file)) > > + > > + # Remote patches won't be present and compressed patches won't be > > + # unpacked, so say we're not scanning them > > + if not os.path.isfile(patch_file): > > + bb.note("%s is remote or compressed, not scanning content" % > > patch_file) > > + return cve_ids > > + > > + with open(patch_file, "r", encoding="utf-8") as f: > > + try: > > + patch_text = f.read() > > + except UnicodeDecodeError: > > + bb.debug( > > + 1, > > + "Failed to read patch %s using UTF-8 encoding" > > + " trying with iso8859-1" % patch_file, > > + ) > > + f.close() > > + with open(patch_file, "r", encoding="iso8859-1") as f: > > + patch_text = f.read() > > + > > + cve_ids.update(parse_cves_from_patch_contents(patch_text)) > > + > > + if not cve_ids: > > + bb.debug(2, "Patch %s doesn't solve CVEs" % patch_file) > > + else: > > + bb.debug(2, "Patch %s solves %s" % (patch_file, ", > > ".join(sorted(cve_ids)))) > > + > > + return cve_ids > > + > > + > > +def get_patched_cves(d): > > + """ > > + Determines the CVE IDs that have been solved by either patches incuded > > within > > + SRC_URI or by setting CVE_STATUS. > > + > > + Returns a dictionary with the CVE IDs as keys and an associated > > dictonary > > of > > + relevant metadata as the value. > > + """ > > patched_cves = {} > > patches = oe.patch.src_patches(d) > > bb.debug(2, "Scanning %d patches for CVEs" % len(patches)) > > + > > + # Check each patch file > > for url in patches: > > patch_file = bb.fetch.decodeurl(url)[2] > > - > > - # Check patch file name for CVE ID > > - fname_match = cve_file_name_match.search(patch_file) > > - if fname_match: > > - cve = fname_match.group(1).upper() > > - patched_cves[cve] = {"abbrev-status": "Patched", "status": > > "fix-file- > > included", "resource": patch_file} > > - bb.debug(2, "Found %s from patch file name %s" % (cve, > > patch_file)) > > - > > - # Remote patches won't be present and compressed patches won't be > > - # unpacked, so say we're not scanning them > > - if not os.path.isfile(patch_file): > > - bb.note("%s is remote or compressed, not scanning content" % > > patch_file) > > - continue > > - > > - with open(patch_file, "r", encoding="utf-8") as f: > > - try: > > - patch_text = f.read() > > - except UnicodeDecodeError: > > - bb.debug(1, "Failed to read patch %s using UTF-8 encoding" > > - " trying with iso8859-1" % patch_file) > > - f.close() > > - with open(patch_file, "r", encoding="iso8859-1") as f: > > - patch_text = f.read() > > - > > - # Search for one or more "CVE: " lines > > - text_match = False > > - for match in cve_match.finditer(patch_text): > > - # Get only the CVEs without the "CVE: " tag > > - cves = patch_text[match.start()+5:match.end()] > > - for cve in cves.split(): > > - bb.debug(2, "Patch %s solves %s" % (patch_file, cve)) > > - patched_cves[cve] = {"abbrev-status": "Patched", "status": > > "fix-file- > > included", "resource": patch_file} > > - text_match = True > > - > > - if not fname_match and not text_match: > > - bb.debug(2, "Patch %s doesn't solve CVEs" % patch_file) > > + for cve_id in parse_cves_from_patch_file(patch_file): > > + if cve_id not in patched_cves: > > + { > > + "abbrev-status": "Patched", > > + "status": "fix-file-included", > > + "resource": [patch_file], > > + } > > + else: > > + patched_cves[cve_id]["resource"].append(patch_file) > > > > # Search for additional patched CVEs > > - for cve in (d.getVarFlags("CVE_STATUS") or {}): > > - decoded_status = decode_cve_status(d, cve) > > + for cve_id in d.getVarFlags("CVE_STATUS") or {}: > > + decoded_status = decode_cve_status(d, cve_id) > > products = d.getVar("CVE_PRODUCT") > > - if has_cve_product_match(decoded_status, products) == True: > > - patched_cves[cve] = { > > + if has_cve_product_match(decoded_status, products): > > + if cve_id in patched_cves: > > + bb.warn( > > + 'CVE_STATUS[%s] = "%s" is overwriting previous status > > of "%s: > %s"' > > + % ( > > + cve_id, > > + d.getVarFlag("CVE_STATUS", cve_id), > > + patched_cves[cve_id]["abbrev-status"], > > + patched_cves[cve_id]["status"], > > + ) > > + ) > > + patched_cves[cve_id] = { > > "abbrev-status": decoded_status["mapping"], > > "status": decoded_status["detail"], > > "justification": decoded_status["description"], > > "affected-vendor": decoded_status["vendor"], > > - "affected-product": decoded_status["product"] > > + "affected-product": decoded_status["product"], > > } > > > > return patched_cves > > diff --git a/meta/lib/oeqa/selftest/cases/cve_check.py > > b/meta/lib/oeqa/selftest/cases/cve_check.py > > index 3dd3e89d3e..511e4b81b4 100644 > > --- a/meta/lib/oeqa/selftest/cases/cve_check.py > > +++ b/meta/lib/oeqa/selftest/cases/cve_check.py > > @@ -120,6 +120,211 @@ class CVECheck(OESelftestTestCase): > > self.assertEqual(has_cve_product_match(status, "test > > glibca:glibc"), > True) > > self.assertEqual(has_cve_product_match(status, "glibca:glibc > > test"), > True) > > > > + def test_parse_cve_from_patch_filename(self): > > + from oe.cve_check import parse_cve_from_filename > > + > > + # Patch filename without CVE ID > > + self.assertEqual(parse_cve_from_filename("0001-test.patch"), "") > > + > > + # Patch with single CVE ID > > + self.assertEqual( > > + parse_cve_from_filename("CVE-2022-12345.patch"), "CVE-2022- > > 12345" > > + ) > > + > > + # Patch with multiple CVE IDs > > + self.assertEqual( > > + parse_cve_from_filename("CVE-2022-41741-CVE-2022- > > 41742.patch"), > > + "CVE-2022-41742", > > + ) > > + > > + # Patches with CVE ID and appended text > > + self.assertEqual( > > + parse_cve_from_filename("CVE-2023-3019-0001.patch"), "CVE- > > 2023-3019" > > + ) > > + self.assertEqual( > > + parse_cve_from_filename("CVE-2024-21886-1.patch"), "CVE-2024- > > 21886" > > + ) > > + > > + # Patch with CVE ID and prepended text > > + self.assertEqual( > > + parse_cve_from_filename("grep-CVE-2012-5667.patch"), "CVE- > 2012- > > 5667" > > + ) > > + self.assertEqual( > > + parse_cve_from_filename("0001-CVE-2012-5667.patch"), "CVE- > > 2012-5667" > > + ) > > + > > + # Patch with CVE ID and both prepended and appended text > > + self.assertEqual( > > + parse_cve_from_filename( > > + "0001-tpm2_import-fix-fixed-AES-key-CVE-2021-3565- > 0001.patch" > > + ), > > + "CVE-2021-3565", > > + ) > > + > > + # Only grab the last CVE ID in the filename > > + self.assertEqual( > > + parse_cve_from_filename("CVE-2012-5667-CVE-2012- > 5668.patch"), > > + "CVE-2012-5668", > > + ) > > + > > + # Test invalid CVE ID with incorrect length (must be at least 4 > > digits) > > + self.assertEqual( > > + parse_cve_from_filename("CVE-2024-001.patch"), > > + "", > > + ) > > + > > + # Test valid CVE ID with very long length > > + self.assertEqual( > > + parse_cve_from_filename("CVE-2024- > > 0000000000000000000000001.patch"), > > + "CVE-2024-0000000000000000000000001", > > + ) > > + > > + def test_parse_cve_from_patch_contents(self): > > + import textwrap > > + from oe.cve_check import parse_cves_from_patch_contents > > + > > + # Standard patch file excerpt without any patches > > + self.assertEqual( > > + parse_cves_from_patch_contents( > > + textwrap.dedent("""\ > > + remove "*" for root since we don't have a /etc/shadow so far. > > + > > + Upstream-Status: Inappropriate [configuration] > > + > > + Signed-off-by: Scott Garman <[email protected]> > > + > > + --- base-passwd/passwd.master~nobash > > + +++ base-passwd/passwd.master > > + @@ -1,4 +1,4 @@ > > + -root:*:0:0:root:/root:/bin/sh > > + +root::0:0:root:/root:/bin/sh > > + daemon:*:1:1:daemon:/usr/sbin:/bin/sh > > + bin:*:2:2:bin:/bin:/bin/sh > > + sys:*:3:3:sys:/dev:/bin/sh > > + """) > > + ), > > + set(), > > + ) > > + > > + # Patch file with multiple CVE IDs (space-separated) > > + self.assertEqual( > > + parse_cves_from_patch_contents( > > + textwrap.dedent("""\ > > + There is an assertion in function > > _cairo_arc_in_direction(). > > + > > + CVE: CVE-2019-6461 CVE-2019-6462 > > + Upstream-Status: Pending > > + Signed-off-by: Ross Burton <[email protected]> > > + > > + diff --git a/src/cairo-arc.c b/src/cairo-arc.c > > + index 390397bae..1bde774a4 100644 > > + --- a/src/cairo-arc.c > > + +++ b/src/cairo-arc.c > > + @@ -186,7 +186,8 @@ _cairo_arc_in_direction (cairo_t > > *cr, > > + if (cairo_status (cr)) > > + return; > > + > > + - assert (angle_max >= angle_min); > > + + if (angle_max < angle_min) > > + + return; > > + > > + if (angle_max - angle_min > 2 * M_PI * > > MAX_FULL_CIRCLES) { > > + angle_max = fmod (angle_max - angle_min, 2 * M_PI); > > + """), > > + ), > > + {"CVE-2019-6461", "CVE-2019-6462"}, > > + ) > > + > > + # Patch file with multiple CVE IDs (comma-separated w/ both space > > and > > no space) > > + self.assertEqual( > > + parse_cves_from_patch_contents( > > + textwrap.dedent("""\ > > + There is an assertion in function > > _cairo_arc_in_direction(). > > + > > + CVE: CVE-2019-6461,CVE-2019-6462, CVE-2019-6463 > > + Upstream-Status: Pending > > + Signed-off-by: Ross Burton <[email protected]> > > + > > + diff --git a/src/cairo-arc.c b/src/cairo-arc.c > > + index 390397bae..1bde774a4 100644 > > + --- a/src/cairo-arc.c > > + +++ b/src/cairo-arc.c > > + @@ -186,7 +186,8 @@ _cairo_arc_in_direction (cairo_t > > *cr, > > + if (cairo_status (cr)) > > + return; > > + > > + - assert (angle_max >= angle_min); > > + + if (angle_max < angle_min) > > + + return; > > + > > + if (angle_max - angle_min > 2 * M_PI * > > MAX_FULL_CIRCLES) { > > + angle_max = fmod (angle_max - angle_min, 2 * M_PI); > > + > > + """), > > + ), > > + {"CVE-2019-6461", "CVE-2019-6462", "CVE-2019-6463"}, > > + ) > > + > > + # Patch file with multiple CVE IDs (&-separated) > > + self.assertEqual( > > + parse_cves_from_patch_contents( > > + textwrap.dedent("""\ > > + There is an assertion in function > > _cairo_arc_in_direction(). > > + > > + CVE: CVE-2019-6461 & CVE-2019-6462 > > + Upstream-Status: Pending > > + Signed-off-by: Ross Burton <[email protected]> > > + > > + diff --git a/src/cairo-arc.c b/src/cairo-arc.c > > + index 390397bae..1bde774a4 100644 > > + --- a/src/cairo-arc.c > > + +++ b/src/cairo-arc.c > > + @@ -186,7 +186,8 @@ _cairo_arc_in_direction (cairo_t > > *cr, > > + if (cairo_status (cr)) > > + return; > > + > > + - assert (angle_max >= angle_min); > > + + if (angle_max < angle_min) > > + + return; > > + > > + if (angle_max - angle_min > 2 * M_PI * > > MAX_FULL_CIRCLES) { > > + angle_max = fmod (angle_max - angle_min, 2 * M_PI); > > + """), > > + ), > > + {"CVE-2019-6461", "CVE-2019-6462"}, > > + ) > > + > > + # Patch file with multiple lines with CVE IDs > > + self.assertEqual( > > + parse_cves_from_patch_contents( > > + textwrap.dedent("""\ > > + There is an assertion in function > > _cairo_arc_in_direction(). > > + > > + CVE: CVE-2019-6461 & CVE-2019-6462 > > + > > + CVE: CVE-2019-6463 & CVE-2019-6464 > > + Upstream-Status: Pending > > + Signed-off-by: Ross Burton <[email protected]> > > + > > + diff --git a/src/cairo-arc.c b/src/cairo-arc.c > > + index 390397bae..1bde774a4 100644 > > + --- a/src/cairo-arc.c > > + +++ b/src/cairo-arc.c > > + @@ -186,7 +186,8 @@ _cairo_arc_in_direction (cairo_t > > *cr, > > + if (cairo_status (cr)) > > + return; > > + > > + - assert (angle_max >= angle_min); > > + + if (angle_max < angle_min) > > + + return; > > + > > + if (angle_max - angle_min > 2 * M_PI * > > MAX_FULL_CIRCLES) { > > + angle_max = fmod (angle_max - angle_min, 2 * M_PI); > > + > > + """), > > + ), > > + {"CVE-2019-6461", "CVE-2019-6462", "CVE-2019-6463", "CVE- > > 2019-6464"}, > > + ) > > > > def test_recipe_report_json(self): > > config = """ > > -- > > 2.34.1
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#209637): https://lists.openembedded.org/g/openembedded-core/message/209637 Mute This Topic: https://lists.openembedded.org/mt/110347357/21656 Group Owner: [email protected] Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
