Merge ~litios/ubuntu-cve-tracker:oval/fix_broken_criteria into ubuntu-cve-tracker:master

Proposed by David Fernandez Gonzalez
Status: Merged
Merge reported by: David Fernandez Gonzalez
Merged at revision: 2ce9e012bd027389b5bf836a2c7e4fc000218f0f
Proposed branch: ~litios/ubuntu-cve-tracker:oval/fix_broken_criteria
Merge into: ubuntu-cve-tracker:master
Diff against target: 173 lines (+42/-22)
1 file modified
scripts/oval_lib.py (+42/-22)
Reviewer Review Type Date Requested Status
Paulo Flabiano Smorigo Approve
Eduardo Barretto Approve
Review via email: mp+457529@code.launchpad.net

Description of the change

This PR fixes several issues:

1. Prevent empty CVEs from being created when the affected packages have no valid binaries.

2. Properly set parent release priorities. A bug occurred where the status wasn't set correctly, making the results unpredictable.

3. Set the right version and status when there are parent releases involved.
   1. If the status is vulnerable, use the top release with the latest available binaries.
   2. If the status is fixed, find the parent in which it was fixed and use that.

To post a comment you must log in.
Revision history for this message
David Fernandez Gonzalez (litios) wrote :

Diffs:

* Jammy: https://pastebin.canonical.com/p/NTjzD4Fkjj/ (None)
* Jammy OCI: https://pastebin.canonical.com/p/h5nvD5RWnP/ (None)
* Bionic: https://pastebin.canonical.com/p/NVXXRq7Hd3/ (Empty CVE gone)
* Bionic OCI: https://pastebin.canonical.com/p/r4jfy6nhnc/ (Emtpy CVE gone)
* ESM-apps Xenial: Too long for pastebin (Many esm-apps/xenial -> xenial, affects kernel too)
  - Results diff: (None)

Revision history for this message
Eduardo Barretto (ebarretto) wrote :

lgtm, thanks for handling/investigating this!

review: Approve
Revision history for this message
Paulo Flabiano Smorigo (pfsmorigo) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/scripts/oval_lib.py b/scripts/oval_lib.py
2index f0c89fc..489200e 100644
3--- a/scripts/oval_lib.py
4+++ b/scripts/oval_lib.py
5@@ -311,15 +311,24 @@ class CVE:
6 continue
7
8 if pkg.name not in pkg_rel:
9- pkg_rel[pkg.name] = pkg.rel
10- elif releases.index(pkg.rel) < releases.index(pkg_rel[pkg.name]):
11- pkg_rel[pkg.name] = pkg.rel
12-
13+ pkg_rel[pkg.name] = pkg
14+ else:
15+ pkg_rel_entry = self.pkg_rel_entries[str(pkg)]
16+ curr_pkg_rel_entry = self.pkg_rel_entries[str(pkg_rel[pkg.name])]
17+ if curr_pkg_rel_entry.status == 'fixed':
18+ priority = releases.index(pkg.rel) > releases.index(pkg_rel[pkg.name].rel) and \
19+ pkg_rel_entry.fixed_version in pkg.versions_binaries
20+ else:
21+ priority = releases.index(pkg.rel) < releases.index(pkg_rel[pkg.name].rel)
22+
23+ if priority:
24+ pkg_rel[pkg.name] = pkg
25+
26 for pkg in self.pkgs:
27 if self.pkg_rel_entries[str(pkg)].is_not_applicable():
28 continue
29
30- if pkg.name in pkg_rel and pkg_rel[pkg.name] == pkg.rel:
31+ if pkg.name in pkg_rel and pkg_rel[pkg.name].rel == pkg.rel:
32 pkgs.append(pkg)
33
34 return pkgs
35@@ -433,11 +442,13 @@ class Package:
36 def get_binaries(self, source_version, binary_version):
37 if not self.versions_binaries: return {}
38 if source_version not in self.versions_binaries:
39- if len(self.versions_binaries[self.earliest_version]) != 1:
40- print(f"WARN: Version {source_version} doesn't exist yet the package {self.name} has different versions for the binaries")
41-
42 version_binaries = self.versions_binaries[self.earliest_version]
43- return version_binaries[self.get_binary_versions(self.earliest_version)[0]]
44+ if len(version_binaries) > 1:
45+ print(f"WARN: Version {source_version} doesn't exist yet the package {self.name} has different versions for the binaries")
46+ elif len(version_binaries) == 0:
47+ return []
48+ binary_versions = self.get_binary_versions(self.earliest_version)
49+ return version_binaries[binary_versions[0]]
50 return self.versions_binaries[source_version][binary_version]
51
52 def __str__(self) -> str:
53@@ -483,12 +494,13 @@ class OvalGenerator:
54 self.release_codename = cve_lib.release_progenitor(self.release) if cve_lib.release_progenitor(self.release) else self.release.replace('/', '_')
55 self.release_name = cve_lib.release_name(self.release)
56
57- self.parent_releases = set()
58+ self.parent_releases = list()
59 current_release = self.release
60 while(cve_lib.release_parent(current_release)):
61 current_release = cve_lib.release_parent(current_release)
62- if current_release != self.release:
63- self.parent_releases.add(current_release)
64+ if current_release != self.release and \
65+ current_release not in self.parent_releases:
66+ self.parent_releases.append(current_release)
67
68 self.ns = 'oval:com.ubuntu.{0}'.format(self.release_codename)
69 self.id = 100
70@@ -1441,7 +1453,7 @@ class OvalGeneratorPkg(OvalGenerator):
71 self._increase_id(is_definition=True)
72
73 all_pkgs = dict()
74- for parent_release in list(self.parent_releases)[::-1]:
75+ for parent_release in self.parent_releases[::-1]:
76 all_pkgs.update(self.packages[parent_release])
77
78 all_pkgs.update(self.packages[self.release])
79@@ -1556,16 +1568,18 @@ class OvalGeneratorCVE(OvalGenerator):
80
81 return instruction
82
83- def _populate_pkg(self, cve: CVE, package: Package, root_element, main_criteria, cache, fixed_versions) -> None:
84+ def _populate_pkg(self, cve: CVE, package: Package, root_element, main_criteria, cache, fixed_versions) -> bool:
85 tests = root_element.find("tests")
86 objects = root_element.find("objects")
87 variables = root_element.find("variables")
88 states = root_element.find("states")
89 pkg_rel_entry = cve.pkg_rel_entries[str(package)]
90+ added = False
91
92 source_version = package.get_version_to_check(pkg_rel_entry.fixed_version)
93 for binary_version in package.get_binary_versions(source_version):
94 binaries = package.get_binaries(source_version, binary_version)
95+ if not binaries: continue
96 cache_entry = f'{package.name}-{binary_version}'
97
98 cache.setdefault(cache_entry, dict(bin_id=None, def_id=None))
99@@ -1578,6 +1592,7 @@ class OvalGeneratorCVE(OvalGenerator):
100 cache_entry_bin = cache_entry
101
102 if pkg_rel_entry.status == 'vulnerable' and not self.fixed_only:
103+ added = True
104 if not cache_entry in cache or not cache[cache_entry]['def_id']:
105 self._add_criterion(self.definition_id, pkg_rel_entry, cve, main_criteria)
106
107@@ -1594,6 +1609,7 @@ class OvalGeneratorCVE(OvalGenerator):
108 else:
109 self._add_criterion(cache[cache_entry]['def_id'], pkg_rel_entry, cve, main_criteria)
110 elif pkg_rel_entry.status == 'fixed':
111+ added = True
112 if binary_version in fixed_versions:
113 self._add_criterion(fixed_versions[binary_version], pkg_rel_entry, cve, main_criteria)
114 else:
115@@ -1611,6 +1627,8 @@ class OvalGeneratorCVE(OvalGenerator):
116 fixed_versions[binary_version] = self.definition_id
117 self._increase_id(is_definition=False)
118
119+ return added
120+
121 def _populate_kernel_pkg(self, cve: CVE, package: Package, root_element, main_criteria, running_kernel_id, cache, fixed_versions) -> None:
122 # Kernel binaries have all same version
123 version = package.get_latest_version()
124@@ -1636,27 +1654,29 @@ class OvalGeneratorCVE(OvalGenerator):
125
126 def _generate_elements_from_cve(self, cve, supported_releases, root_element, running_kernel_id, pkg_cache, fixed_versions) -> None:
127 if not cve.pkgs: return
128- added = False
129+ cve_added = False
130 definition_element = self._generate_definition_object(cve)
131 instructions = ''
132 pkgs = cve.get_pkgs(supported_releases)
133 for pkg in pkgs:
134+ pkg_added = False
135 if not pkg.versions_binaries: continue
136- if not pkg.get_binary_versions(next(iter(pkg.versions_binaries))): continue
137 if self._ignore_source_package(pkg.name): continue
138
139- added = True
140 pkg_rel_entry = cve.pkg_rel_entries[str(pkg)]
141 if pkg.is_kernel_pkg and self.oval_format != 'oci':
142 self._populate_kernel_pkg(cve, pkg, root_element, definition_element, running_kernel_id, pkg_cache, fixed_versions)
143+ pkg_added = True
144 else:
145- self._populate_pkg(cve, pkg, root_element, definition_element, pkg_cache, fixed_versions)
146+ pkg_added = self._populate_pkg(cve, pkg, root_element, definition_element, pkg_cache, fixed_versions)
147
148- if pkg_rel_entry.status == 'fixed' and pkg.versions_binaries:
149+ if pkg_rel_entry.status == 'fixed' and pkg_added:
150 product_description = cve_lib.get_subproject_description(pkg_rel_entry.release)
151 instructions = self.prepare_instructions(instructions, cve, product_description, pkg, pkg_rel_entry.fixed_version)
152+
153+ cve_added = cve_added | pkg_added
154
155- if added:
156+ if cve_added:
157 definitions = root_element.find("definitions")
158 metadata = definition_element.find('metadata')
159 metadata.find('description').text = metadata.find('description').text + instructions
160@@ -1681,11 +1701,11 @@ class OvalGeneratorCVE(OvalGenerator):
161
162 pkg_cache = {}
163 fixed_versions = {}
164- accepted_releases = list(self.parent_releases)
165+ accepted_releases = self.parent_releases.copy()
166 accepted_releases.insert(0, self.release)
167
168 all_cves = self.cves[self.release]
169- for parent_release in list(self.parent_releases):
170+ for parent_release in self.parent_releases:
171 for cve in self.cves[parent_release]:
172 if cve not in all_cves:
173 all_cves[cve] = self.cves[parent_release][cve]

Subscribers

People subscribed via source and target branches