Merge ~alexmurray/ubuntu-cve-tracker:misc-cve-lib-and-check-syntax-cleanups into ubuntu-cve-tracker:master

[Alex Murray]

A bunch of miscellaneous fixups to get some bits of check-syntax working betterer

[Seth Arnold (seth-arnold)]

I added one comment on the /proc/pid/fd/0 symlink-breaking -- and did a very quick skim on the rest; it seemed fine, but also like the sort of mechanical thing that would be easy to overlook a mistake.

Thanks

[Alex Murray (alexmurray)]

Thanks Seth - not sure I understand your comment so if you could help explain it for me that would be great.

[Eduardo Barretto (ebarretto)]

hey Alex, is this still needed?

review: Needs Information

[Alex Murray (alexmurray)]

Added --stdin as first class argument to check-syntax and fixed up other bits. Yes this is still needed. As can be seen the check-syntax lpci job fails as expected currently showing check-syntax still works for the regular use-case as well as this new --stdin one (which is used by the flymake integration for emacs in cve-mode.el)

[Alex Murray (alexmurray)]

Any chance to get a review on this? Otherwise I'll look to merge it tomorrow during CVE triage.

[Eduardo Barretto (ebarretto)]

cve_lib and check-syntax changes lgtm!
thanks!

review: Approve

diff --git a/scripts/check-syntax b/scripts/check-syntax
index f89a0e4..7d401f6 100755
--- a/scripts/check-syntax
+++ b/scripts/check-syntax
@@ -261,6 +261,13 @@ parser.add_option(
     help="Number of jobs to run simultaneously (default: %d)" % multiprocessing.cpu_count(),
     default=multiprocessing.cpu_count(),
 )
+parser.add_option(
+    "--stdin",
+    help="Check the contents of stdin instead of a filelist or modified files etc",
+    action="store_true",
+    default=False,
+)
+
 # parser.add_option("-c", "--cna", help="Ensure every CVE assigned by Canonical's CNA has an entry", action='store_true')
 (opt, args) = parser.parse_args()
 
@@ -304,7 +311,11 @@ if os.path.islink(cve_lib.embargoed_dir):
 debug("check_dirs %s" % check_dirs)
 
 all_files = True
-if len(args) == 0:
+if opt.stdin:
+    debug("Using /dev/stdin as input")
+    args = ["/dev/stdin"]
+    all_files = False
+elif len(args) == 0:
     if opt.filelist:
         debug("Using filelist %s" % opt.filelist)
 
@@ -525,6 +536,9 @@ def check_cve(cve):
     except ValueError as e:
         print(e, file=sys.stderr)
         return False
+    # get the real CVE if we are using stdin
+    if opt.stdin:
+        cve = data["Candidate"]
     if cve in ignored:
         print(
             "%s: %d: duplicate CVE found in not-for-us.txt" % (cvepath, 1),
diff --git a/scripts/cve-mode.el b/scripts/cve-mode.el
index b8d8230..4f5797e 100644
--- a/scripts/cve-mode.el
+++ b/scripts/cve-mode.el
@@ -683,7 +683,7 @@ cross boundaries of block literals."
         :name "cve-mode-flymake" :noquery t :connection-type 'pipe
         ;; Make output go to a temporary buffer.
         :buffer (generate-new-buffer " *cve-mode-flymake*")
-        :command `(,cve-mode--check-syntax-executable "/dev/stdin")
+        :command `(,cve-mode--check-syntax-executable "--stdin")
         :sentinel
         (lambda (proc _event)
           (when (memq (process-status proc) '(exit signal))
@@ -723,7 +723,7 @@ cross boundaries of block literals."
 
 ;;;###autoload
 (defun cve-mode-setup-flymake-backend ()
-  "Setuo the `flymake' backend for `cve-mode'."
+  "Setup the `flymake' backend for `cve-mode'."
   (add-hook 'flymake-diagnostic-functions 'cve-mode-flymake nil t))
 
 ;;;###autoload
diff --git a/scripts/cve_lib.py b/scripts/cve_lib.py
index 416d5f7..811ea55 100755
--- a/scripts/cve_lib.py
+++ b/scripts/cve_lib.py
@@ -980,8 +980,8 @@ def find_external_subproject_cves(cve):
         # top-level project directory even though this is preferred
         for d in [get_external_subproject_cve_dir(rel),
                   get_external_subproject_dir(rel)]:
-            path = os.path.join(d, cve)
-            if os.path.exists(path):
+            path = os.path.realpath(os.path.join(d, cve))
+            if os.path.exists(path) and path not in cves:
                 cves.append(path)
     return cves
 
@@ -1076,24 +1076,26 @@ def load_external_subprojects(strict=False):
             if project:
                 subprojects[subproject].setdefault("customer", project)
 
-load_external_subprojects()
+    # now ensure they are consistent
+    global devel_release
+    for release in subprojects:
+        details = subprojects[release]
+        rel = release_alias(release)
+        # prefer the alias name
+        all_releases.append(rel)
+        if details["eol"]:
+            eol_releases.append(rel)
+        if "devel" in details and details["devel"]:
+            if devel_release != "":
+                raise ValueError("there can be only one ⚔ devel")
+            devel_release = rel
+        # ubuntu specific releases
+        product, _ = product_series(release)
+        if product == PRODUCT_UBUNTU:
+            releases.append(rel)
 
-for release in subprojects:
-    details = subprojects[release]
-    rel = release_alias(release)
-    # prefer the alias name
-    all_releases.append(rel)
-    if details["eol"]:
-        eol_releases.append(rel)
-    if "devel" in details and details["devel"]:
-        if devel_release != "":
-            raise ValueError("there can be only one ⚔ devel")
-        devel_release = rel
-    # ubuntu specific releases
-    product, series = product_series(release)
-    if product == PRODUCT_UBUNTU:
-        releases.append(rel)
 
+load_external_subprojects()
 
 def release_sort(release_list):
     '''takes a list of release names and sorts them in release order
@@ -2099,7 +2101,7 @@ def find_cve(cve):
 # e.g.: git/github.com/gogo/protobuf_gogoprotobuf: needs-triage
 # This method should keep supporting existing current format:
 # e.g.: bionic_jackson-databind: needs-triage
-def parse_cve_release_package_field(cve, field, data, value, code, msg, linenum):
+def parse_cve_release_package_field(cvefile, field, data, value, code, msg, linenum):
     package = ""
     release = ""
     state = ""
@@ -2107,14 +2109,14 @@ def parse_cve_release_package_field(cve, field, data, value, code, msg, linenum)
     try:
         release, package = field.split('_', 1)
     except ValueError:
-        msg += "%s: %d: bad field with '_': '%s'\n" % (cve, linenum, field)
+        msg += "%s: %d: bad field with '_': '%s'\n" % (cvefile, linenum, field)
         code = EXIT_FAIL
         return False, package, release, state, details, code, msg
 
     try:
         info = value.split(' ', 1)
     except ValueError:
-        msg += "%s: %d: missing state for '%s': '%s'\n" % (cve, linenum, field, value)
+        msg += "%s: %d: missing state for '%s': '%s'\n" % (cvefile, linenum, field, value)
         code = EXIT_FAIL
         return False, package, release, state, details, code, msg
 
@@ -2128,7 +2130,7 @@ def parse_cve_release_package_field(cve, field, data, value, code, msg, linenum)
         details = info[1].strip()
 
     if details.startswith("["):
-        msg += "%s: %d: %s has details that starts with a bracket: '%s'\n" % (cve, linenum, field, details)
+        msg += "%s: %d: %s has details that starts with a bracket: '%s'\n" % (cvefile, linenum, field, details)
         code = EXIT_FAIL
         return False, package, release, state, details, code, msg
 
@@ -2144,19 +2146,19 @@ def parse_cve_release_package_field(cve, field, data, value, code, msg, linenum)
 
     valid_states = ['needs-triage', 'needed', 'active', 'pending', 'released', 'deferred', 'DNE', 'ignored', 'not-affected']
     if state not in valid_states:
-        msg += "%s: %d: %s has unknown state: '%s' (valid states are: %s)\n" % (cve, linenum, field, state,
+        msg += "%s: %d: %s has unknown state: '%s' (valid states are: %s)\n" % (cvefile, linenum, field, state,
                                                                                    ' '.join(valid_states))
         code = EXIT_FAIL
         return False, package, release, state, details, code, msg
 
     # Verify "released" kernels have version details
     #if state == 'released' and package in kernel_srcs and details == '':
-    #    msg += "%s: %s_%s has state '%s' but lacks version note\n" % (cve, package, release, state)
+    #    msg += "%s: %s_%s has state '%s' but lacks version note\n" % (cvefile, package, release, state)
     #    code = EXIT_FAIL
 
     # Verify "active" states have an Assignee
     if state == 'active' and data['Assigned-to'].strip() == "":
-        msg += "%s: %d: %s has state '%s' but lacks 'Assigned-to'\n" % (cve, linenum, field, state)
+        msg += "%s: %d: %s has state '%s' but lacks 'Assigned-to'\n" % (cvefile, linenum, field, state)
         code = EXIT_FAIL
         return False, package, release, state, details, code, msg
 
@@ -2247,16 +2249,25 @@ def amend_external_subproject_pkg(cve, data, srcmap, amendments, code, msg):
             return code, msg
 
         if '_' in field:
-            success, pkg, release, state, details, code, msg = parse_cve_release_package_field(cve, field, data, value, code, msg, linenum)
+            success, pkg, rel, state, details, code, msg = parse_cve_release_package_field(cve, field, data, value, code, msg, linenum)
             if not success:
                 return code, msg
 
+            canon, _, _, _ = get_subproject_details(rel)
+            if canon is None and rel not in ['upstream', 'devel']:
+                msg += "%s: %d: unknown entry '%s'\n" % (cve, linenum, rel)
+                code = EXIT_FAIL
+                return code, msg
             data.setdefault("pkgs", dict())
             data["pkgs"].setdefault(pkg, dict())
             srcmap["pkgs"].setdefault(pkg, dict())
-            # override existing release info if it exists
-            data["pkgs"][pkg][release] = [state, details]
-            srcmap["pkgs"][pkg][release] = (cve, linenum)
+            if rel in data["pkgs"][pkg]:
+                msg += ("%s: %d: duplicate entry for '%s': original at %s line %d (%s)\n"
+                        % (cve, linenum, rel, srcmap['pkgs'][pkg][rel][0], srcmap['pkgs'][pkg][rel][1], data["pkgs"][pkg][rel]))
+                code = EXIT_FAIL
+                return code, msg
+            data["pkgs"][pkg][rel] = [state, details]
+            srcmap["pkgs"][pkg][rel] = (cve, linenum)
 
     return code, msg
 
@@ -2271,7 +2282,7 @@ def load_external_subproject_cve_data(cve, data, srcmap, code, msg):
 
     return code, msg
 
-def load_cve(cve, strict=False, srcmap=None):
+def load_cve(cvefile, strict=False, srcmap=None):
     '''Loads a given CVE into:
        dict( fields...
              'pkgs' -> dict(  pkg -> dict(  release ->  (state, details)   ) )
@@ -2298,13 +2309,13 @@ def load_cve(cve, strict=False, srcmap=None):
     affected = dict()
     lastfield = ""
     fields_seen = []
-    if not os.path.exists(cve):
-        raise ValueError("File does not exist: '%s'" % (cve))
+    if not os.path.exists(cvefile):
+        raise ValueError("File does not exist: '%s'" % cvefile)
     linenum = 0
     notes_parser = NotesParser()
     priority_reason = {}
     cvss_entries = []
-    with codecs.open(cve, encoding="utf-8") as inF:
+    with codecs.open(cvefile, encoding="utf-8") as inF:
         lines = inF.readlines()
     for line in lines:
         line = line.rstrip()
@@ -2317,7 +2328,7 @@ def load_cve(cve, strict=False, srcmap=None):
             try:
                 # parse Notes properly
                 if lastfield == 'Notes':
-                    code, newmsg = notes_parser.parse_line(cve, line, linenum, code)
+                    code, newmsg = notes_parser.parse_line(cvefile, line, linenum, code)
                     if code != EXIT_OKAY:
                         msg += newmsg
                 elif lastfield.startswith('Priority'):
@@ -2333,9 +2344,9 @@ def load_cve(cve, strict=False, srcmap=None):
                         patch_type = patch_type.strip()
                         entry = entry.strip()
                         data['patches'][pkg].append((patch_type, entry))
-                        srcmap['patches'][pkg].append((cve, linenum))
+                        srcmap['patches'][pkg].append((cvefile, linenum))
                     except Exception as e:
-                        msg += "%s: %d: Failed to parse '%s' entry %s: %s\n" % (cve, linenum, lastfield, line, e)
+                        msg += "%s: %d: Failed to parse '%s' entry %s: %s\n" % (cvefile, linenum, lastfield, line, e)
                         code = EXIT_FAIL
                 elif lastfield == 'CVSS':
                     try:
@@ -2358,36 +2369,36 @@ def load_cve(cve, strict=False, srcmap=None):
                         # to a dict first if needed
                         if type(srcmap["CVSS"]) is tuple:
                             srcmap["CVSS"] = dict()
-                        srcmap["CVSS"].setdefault(cvss['source'], (cve, linenum))
+                        srcmap["CVSS"].setdefault(cvss['source'], (cvefile, linenum))
                     except Exception as e:
-                        msg += "%s: %d: Failed to parse CVSS: %s\n" % (cve, linenum, e)
+                        msg += "%s: %d: Failed to parse CVSS: %s\n" % (cvefile, linenum, e)
                         code = EXIT_FAIL
                 else:
                     data[lastfield] += '\n%s' % (line[1:])
             except KeyError as e:
-                msg += "%s: %d: bad line '%s' (%s)\n" % (cve, linenum, line, e)
+                msg += "%s: %d: bad line '%s' (%s)\n" % (cvefile, linenum, line, e)
                 code = EXIT_FAIL
             continue
 
         try:
             field, value = line.split(':', 1)
         except ValueError as e:
-            msg += "%s: %d: bad line '%s' (%s)\n" % (cve, linenum, line, e)
+            msg += "%s: %d: bad line '%s' (%s)\n" % (cvefile, linenum, line, e)
             code = EXIT_FAIL
             continue
 
         lastfield = field = field.strip()
         if field in fields_seen:
-            msg += "%s: %d: repeated field '%s'\n" % (cve, linenum, field)
+            msg += "%s: %d: repeated field '%s'\n" % (cvefile, linenum, field)
             code = EXIT_FAIL
         else:
             fields_seen.append(field)
         value = value.strip()
         if field == 'Candidate':
             data.setdefault(field, value)
-            srcmap.setdefault(field, (cve, linenum))
+            srcmap.setdefault(field, (cvefile, linenum))
             if value != "" and not value.startswith('CVE-') and not value.startswith('UEM-') and not value.startswith('EMB-'):
-                msg += "%s: %d: unknown Candidate '%s' (must be /(CVE|UEM|EMB)-/)\n" % (cve, linenum, value)
+                msg += "%s: %d: unknown Candidate '%s' (must be /(CVE|UEM|EMB)-/)\n" % (cvefile, linenum, value)
                 code = EXIT_FAIL
         elif 'Priority' in field:
             # For now, throw away comments on Priority fields
@@ -2397,26 +2408,26 @@ def load_cve(cve, strict=False, srcmap=None):
                 try:
                     _, pkg = field.split('_', 1)
                 except ValueError:
-                    msg += "%s: %d: bad field with 'Priority_': '%s'\n" % (cve, linenum, field)
+                    msg += "%s: %d: bad field with 'Priority_': '%s'\n" % (cvefile, linenum, field)
                     code = EXIT_FAIL
                     continue
             # initially set the priority reason as an empty string - this will
             # be fixed up later with a real value if one is found
             data.setdefault(field, [value, ""])
-            srcmap.setdefault(field, (cve, linenum))
+            srcmap.setdefault(field, (cvefile, linenum))
             if value not in ['untriaged', 'not-for-us'] + priorities:
-                msg += "%s: %d: unknown Priority '%s'\n" % (cve, linenum, value)
+                msg += "%s: %d: unknown Priority '%s'\n" % (cvefile, linenum, value)
                 code = EXIT_FAIL
         elif 'Patches_' in field:
             try:
                 _, pkg = field.split('_', 1)
             except ValueError:
-                msg += "%s: %d: bad field with 'Patches_': '%s'\n" % (cve, linenum, field)
+                msg += "%s: %d: bad field with 'Patches_': '%s'\n" % (cvefile, linenum, field)
                 code = EXIT_FAIL
                 continue
             # value should be empty
             if len(value) > 0:
-                msg += "%s: %d: '%s' field should have no value\n" % (cve, linenum, field)
+                msg += "%s: %d: '%s' field should have no value\n" % (cvefile, linenum, field)
                 code = EXIT_FAIL
                 continue
             data['patches'].setdefault(pkg, list())
@@ -2426,41 +2437,42 @@ def load_cve(cve, strict=False, srcmap=None):
             try:
                 _, pkg = field.split('_', 1)
             except ValueError:
-                msg += "%s: %d: bad field with 'Tags_': '%s'\n" % (cve, linenum, field)
+                msg += "%s: %d: bad field with 'Tags_': '%s'\n" % (cvefile, linenum, field)
                 code = EXIT_FAIL
                 continue
             data['tags'].setdefault(pkg, set())
-            srcmap['tags'].setdefault(pkg, (cve, linenum))
+            srcmap['tags'].setdefault(pkg, (cvefile, linenum))
             for word in value.strip().split(' '):
                 if word not in valid_tags:
-                    msg += "%s: %d: invalid tag '%s': '%s'\n" % (cve, linenum, word, field)
+                    msg += "%s: %d: invalid tag '%s': '%s'\n" % (cvefile, linenum, word, field)
                     code = EXIT_FAIL
                     continue
                 data['tags'][pkg].add(word)
         elif '_' in field:
-            success, pkg, rel, state, details, code, msg = parse_cve_release_package_field(cve, field, data, value, code, msg, linenum)
+            success, pkg, rel, state, details, code, msg = parse_cve_release_package_field(cvefile, field, data, value, code, msg, linenum)
             if not success:
                 assert(code == EXIT_FAIL)
                 continue
             canon, _, _, _ = get_subproject_details(rel)
             if canon is None and rel not in ['upstream', 'devel']:
-                msg += "%s: %d: unknown entry '%s'\n" % (cve, linenum, rel)
+                msg += "%s: %d: unknown entry '%s'\n" % (cvefile, linenum, rel)
                 code = EXIT_FAIL
                 continue
             affected.setdefault(pkg, dict())
             if rel in affected[pkg]:
-                msg += "%s: %d: duplicate entry for '%s': original at line %d\n" % (cve, linenum, rel, srcmap['pkgs'][pkg][rel][1])
+                msg += ("%s: %d: duplicate entry for '%s': original at %s line %d\n"
+                        % (cvefile, linenum, rel, srcmap['pkgs'][pkg][rel][0], srcmap['pkgs'][pkg][rel][1]))
                 code = EXIT_FAIL
                 continue
             affected[pkg].setdefault(rel, [state, details])
             srcmap['pkgs'].setdefault(pkg, dict())
-            srcmap['pkgs'][pkg].setdefault(rel, (cve, linenum))
+            srcmap['pkgs'][pkg].setdefault(rel, (cvefile, linenum))
         elif field not in required_fields + extra_fields:
-            msg += "%s: %d: unknown field '%s'\n" % (cve, linenum, field)
+            msg += "%s: %d: unknown field '%s'\n" % (cvefile, linenum, field)
             code = EXIT_FAIL
         else:
             data.setdefault(field, value)
-            srcmap.setdefault(field, (cve, linenum))
+            srcmap.setdefault(field, (cvefile, linenum))
 
     data['Notes'] = notes_parser.finalize()
     data['CVSS'] = cvss_entries
@@ -2468,28 +2480,28 @@ def load_cve(cve, strict=False, srcmap=None):
     # Check for required fields
     for field in required_fields:
         # boilerplate files are special and can (should?) be empty
-        nonempty = [] if "boilerplate" in cve else ['Candidate']
+        nonempty = [] if "boilerplate" in cvefile else ['Candidate']
         if strict:
             nonempty += ['PublicDate']
 
         if field not in data or field not in fields_seen:
-            msg += "%s: %d: missing field '%s'\n" % (cve, linenum, field)
+            msg += "%s: %d: missing field '%s'\n" % (cvefile, linenum, field)
             code = EXIT_FAIL
         elif field in nonempty and data[field].strip() == "":
-            msg += "%s: %d: required field '%s' is empty\n" % (cve, linenum, field)
+            msg += "%s: %d: required field '%s' is empty\n" % (cvefile, linenum, field)
             code = EXIT_FAIL
 
     # Fill in defaults for missing fields
     if 'Priority' not in data:
         data.setdefault('Priority', ['untriaged'])
-        srcmap.setdefault('Priority', (cve, 1))
+        srcmap.setdefault('Priority', (cvefile, 1))
     # Perform override fields
     if 'PublicDateAtUSN' in data:
         data['PublicDate'] = data['PublicDateAtUSN']
         srcmap['PublicDate'] = srcmap['PublicDateAtUSN']
     if 'CRD' in data and data['CRD'].strip() != '' and data['PublicDate'] != data['CRD']:
-        if cve.startswith("embargoed"):
-            print("%s: %d: adjusting PublicDate to use CRD: %s" % (cve, linenum, data['CRD']), file=sys.stderr)
+        if cvefile.startswith("embargoed"):
+            print("%s: %d: adjusting PublicDate to use CRD: %s" % (cvefile, linenum, data['CRD']), file=sys.stderr)
         data['PublicDate'] = data['CRD']
         srcmap['PublicDate'] = srcmap['CRD']
 
@@ -2510,12 +2522,12 @@ def load_cve(cve, strict=False, srcmap=None):
             if rel not in external_releases:
                 needs_upstream = True
         if needs_upstream and 'upstream' not in affected[pkg]:
-            msg += "%s: %d: missing upstream '%s'\n" % (cve, linenum, pkg)
+            msg += "%s: %d: missing upstream '%s'\n" % (cvefile, linenum, pkg)
             code = EXIT_FAIL
 
     data['pkgs'] = affected
 
-    code, msg = load_external_subproject_cve_data(cve, data, srcmap, code, msg)
+    code, msg = load_external_subproject_cve_data(cvefile, data, srcmap, code, msg)
 
     if code != EXIT_OKAY:
         raise ValueError(msg.strip())