Merge ~alexmurray/ubuntu-cve-tracker:mistriage-as-cve-triage into ubuntu-cve-tracker:master

[Alex Murray]

[Mike Salvatore (mikesalvatore)]

The ignore-mistriaged.txt list gives me pause. I'd like to have some kind of process/policy/strategy in place to try to avoid needing to do a mis-mistriaged feature in 5 years. I think it's the most reasonable solution, but we need to avoid becoming trigger-happy with the "ignore" button when processing mistriaged CVEs.

[Seth Arnold (seth-arnold)]

I'm a bit concerned about the runtime of the thing, since the standalone
script takes a while on my system:

$ time ./scripts/report-mistriaged-cves.py
Loading /home/sarnold/Canonical/debian-secure-testing/data/CVE/list ...
CVE-2020-8034 ['php-horde-gollem'] # Horde Groupware Webmail
[.. many lines snipped ..]
CVE-2002-0392 ['apache2'] # apache2, fixed pre-Ubuntu
CVE-2002-0391 ['acm', 'glibc', 'dietlibc', 'krb5', 'openafs'] # krb5, fixed pre-Ubuntu

real 8m45.907s
user 4m29.767s
sys 4m15.820s

Is the probabilistic cve selection (neat idea) going to do a bunch of work
and then discard some of it?

Thanks

[Alex Murray (alexmurray)]

Most of the runtime of the standalone script is in loading the Debian CVEs - since we already do that for check-cves anyway that shouldn't be a problem. The probabalistic calculations are pretty simple - doing a quick benchmark to run that code 100000 times and using a year of 2004 (which means the test will be almost always False) shows it takes 0.06s - so even if we are looking at 100000 CVEs it will only add less than a tenth of a second - so I don't think that should be a problem either :)

#!/usr/bin/env python3
import datetime
import math
import random
import timeit

def benchmark():
   i = 0
   while i < 100000:
     year = 2004
     now = datetime.datetime.utcnow().year
     prob = 1.0 / math.pow(2, now - year)
     rand = random.random()
     if rand > prob:
       pass
     i = i + 1

print(timeit.timeit("benchmark()", setup="from __main__ import benchmark", number=10)/10)

15ae1b6... by Alex Murray on 2020-07-27

scripts/check-cves: Clarify --mistriaged docstring

d3d003b... by Alex Murray on 2020-07-27

scripts/check-cves: Collect all mistriaged CVEs and whittle down

This cleans up the logic to address feedback from mikesalvatore in
https://code.launchpad.net/~alexmurray/ubuntu-cve-tracker/+git/ubuntu-cve-tracker-1/+merge/387806

186021d... by Alex Murray on 2020-07-27

scripts/check-cves: Minor cleanups to doc string placement and style

[Alex Murray (alexmurray)]

See updated commits.

ee53fab... by Alex Murray on 2020-07-27

scripts/check-cves: Fix up comment to match code

[Mike Salvatore (mikesalvatore)]

I like the probability idea. There is the potential, however, that it causes the script to behave differently than inspected. Consider the contrived case where there are 0 2020 mistriaged CVEs, 10 2019 mistriaged CVEs, and no earlier mistriaged CVEs. If I pass `--mistriaged 10` to the script, roughly 5 mistriaged CVEs will be presented to me since they will be selected with probability 1/2. Even though there are 10 mistriaged CVEs and I asked for 10, I only get to evaluate 5.

I don't think this should block the merge; it's just an edge case to consider.

[Alex Murray (alexmurray)]

Mike I agree that this is something that should be done to improve this later down the track. Since there have been no other objections, merging this as is. Will add improvements in further merge requests.

review: Approve

diff --git a/scripts/check-cves b/scripts/check-cves
index 5855daf..4696327 100755
--- a/scripts/check-cves
+++ b/scripts/check-cves
@@ -17,9 +17,11 @@
 
 import datetime
 import json
+import math
 import optparse
 import os
 import os.path
+import random
 import re
 import shutil
 import subprocess
@@ -55,6 +57,9 @@ parser.add_option("--import-missing-debian", help="Process missing Debian CVEs",
 parser.add_option("--debug", help="Report debugging information", action="store_true")
 parser.add_option("--cve", help="Check only the listed comma-separated CVEs and ignore others", action="store",
                   default="")
+parser.add_option("--mistriaged", help="Process the specified number of possible mistriaged CVEs compared to Debian\n"
+                  "Implies --import-missing-debian",
+                  action="store", type=int, default=0)
 (opt, args) = parser.parse_args()
 
 experimental = False
@@ -71,6 +76,8 @@ destdir = "."
 # Skip stuff older than 2005
 cve_limit = 2004
 
+mistriaged_hint = 'Previously triaged as ignored in Ubuntu\n\n'
+
 ignore_strings = [
     "** REJECT **", "Internet Explorer", "Microsoft Edge", "Windows 98",
     "Windows 2000", "Windows XP", "Windows Server 2003", "Windows NT",
@@ -323,6 +330,24 @@ def import_debian(handler):
     today = datetime.date.today()
     known = set(CVEKnownList + CVEIgnoreList)
 
+    def mistriaged(cve):
+        if cve in CVEIgnoreNotForUsList and \
+           cve not in CVEIgnoreMistriagedList and \
+           handler.debian[cve]['state'] == 'FOUND':
+            # check that at least one of the assigned packages exist
+            # in Ubuntu
+            pkgs = {}
+            for pkg in handler.debian[cve]['pkgs'].keys():
+                # ignore package if debian says is not affected
+                if handler.debian[cve]['pkgs'][pkg]['state'] == '<not-affected>':
+                    continue
+                answer = source_map.madison(source, pkg)
+                if len(answer) > 0:
+                    pkgs[pkg] = answer
+            if len(pkgs) > 0:
+                return True
+        return False
+
     # pull in CVEs from data/DSA/list
     dsas = cve_lib.load_debian_dsas(cve_lib.config['secure_testing_path'] + '/data/DSA/list', opt.verbose)
     for dsa in dsas:
@@ -336,9 +361,14 @@ def import_debian(handler):
             if year < cve_limit:
                 continue
 
-            # If we already know about the CVE, skip it
+            # If we already know about the CVE, skip it unless is
+            # mistriaged
             if cve in known:
-                continue
+                if mistriaged(cve):
+                    # add a note about how this was originally classified
+                    dsas[dsa]['desc'] = mistriaged_hint + dsas[dsa]['desc']
+                else:
+                    continue
 
             cves[cve] = dict()
             cves[cve]['subject'] = escape(dsas[dsa]['desc'])
@@ -368,11 +398,15 @@ def import_debian(handler):
                 print("Skipping %s, year %d predates %d" % (cve, year, cve_limit), file=sys.stderr)
             continue
 
-        # If we already know about the CVE, skip it
+        # If we already know about the CVE, skip it unless is mistriaged
         if cve in known:
-            if opt.verbose:
-                print("Skipping %s, already known" % cve, file=sys.stderr)
-            continue
+            if mistriaged(cve):
+                # add a note about how this was originally classified
+                handler.debian[cve]['desc'] = mistriaged_hint + handler.debian[cve]['desc']
+            else:
+                if opt.verbose:
+                    print("Skipping %s, already known" % cve, file=sys.stderr)
+                continue
 
         if handler.debian[cve]['desc'] or handler.debian[cve]['state'] == 'FOUND':
             cves[cve] = dict()
@@ -760,8 +794,8 @@ class CVEHandler(xml.sax.handler.ContentHandler):
             self.curr_refs += [(self.curr_source, self.curr_chars, self.curr_url)]
 
     def handle_cve(self):
-        # Skip CVEs we know about already
-        if self.curr_cve in self.cve_ignore:
+        # Skip CVEs we know about already unless this is a mistriaged CVE
+        if self.curr_cve in self.cve_ignore and mistriaged_hint not in self.curr_desc:
             return
 
         limit = cve_limit
@@ -1261,6 +1295,13 @@ class CVEHandler(xml.sax.handler.ContentHandler):
         print('%s %s %s\n%s' % (action, cve, data, desc))
 
     def add_cve(self, cve, packages, priority=None):
+            # remove from not-for-us.txt if adding and ensure we remove any
+            # mistriaged_hint from the description
+            if cve in CVEIgnoreNotForUsList:
+                cmd = ['sed', '-i', '/^%s #.*$/d' % cve, './ignored/not-for-us.txt']
+                subprocess.call(cmd)
+                self.cve_data[cve]['desc'] = self.cve_data[cve]['desc'].replace(mistriaged_hint, '')
+
             # Build up list of reference urls
             ref_urls = []
             if self.debian and \
@@ -1365,8 +1406,10 @@ class CVEHandler(xml.sax.handler.ContentHandler):
         self.num_added += 1
 
     def ignore_cve(self, cve, reason):
-        # Append to ignore list
-        with open('%s/ignored/not-for-us.txt' % (destdir), 'a') as f:
+        # Append to ignore list unless is already in CVEIgnoreList and then
+        # append to the ignored/ignore-mistriaged.txt
+        txtfile = 'ignore-mistriaged.txt' if cve in CVEIgnoreNotForUsList else 'not-for-us.txt'
+        with open('%s/ignored/%s' % (destdir, txtfile), 'a') as f:
             f.write('%s # %s\n' % (cve, reason))
 
         self.num_ignored += 1
@@ -1459,19 +1502,32 @@ class CheckCVETest(unittest.TestCase):
         self.assertEqual("Exquisite Ultimate Newspaper theme for WordPress", h.get_ignore_suggestion('''The Exquisite Ultimate Newspaper theme 1.3.3 for WordPress has XSS via the anchor identifier to assets/js/jquery.foundation.plugins.js.'''))
 
 
+ignored_notforus_path = 'ignored/not-for-us.txt'
+if destdir != './' and destdir != '.':
+    ignored_notforus_path = os.path.join(destdir, ignored_notforus_path)
+# CVEIgnoreNotForUsList is a list of all CVEs that we have previously
+# chosen to ignore since they don't apply to software in Ubuntu
+CVEIgnoreNotForUsList = cve_lib.parse_CVEs_from_uri(ignored_notforus_path)
+
+ignored_mistriaged_path = 'ignored/ignore-mistriaged.txt'
+if destdir != './' and destdir != '.':
+    ignored_mistriaged_path = os.path.join(destdir, ignored_mistriaged_path)
+# CVEIgnoreMistriagedList is a list of all CVEs that we want to definitely
+# ignore when doing mistriaged CVE detection - they should exist in both
+# CVEIgnoreNotForUsList and CVEIgnoreMistriagedList
+CVEIgnoreMistriagedList = cve_lib.parse_CVEs_from_uri(ignored_mistriaged_path)
+
 # CVEIgnoreList is a list of all CVEs we know about already.  These will be
 # ignored when checking MITRE for new CVEs
-ignored_path = 'ignored/not-for-us.txt'
-if destdir != './' and destdir != '.':
-    ignored_path = os.path.join(destdir, ignored_path)
-CVEIgnoreList = cve_lib.parse_CVEs_from_uri(ignored_path)
+CVEIgnoreList = list(CVEIgnoreNotForUsList)
+
 CVEKnownList = []
 CVEKnownList += [cve for cve in os.listdir(destdir + "/ignored/") if cve.startswith('CVE-')]
 CVEKnownList += [cve for cve in os.listdir(destdir + "/retired/") if cve.startswith('CVE-')]
 (ActiveList, EmbargoList) = cve_lib.get_cve_list()
 CVEKnownList += [cve for cve in ActiveList if cve not in EmbargoList]
 
-if not opt.refresh:
+if not opt.refresh and not opt.mistriaged:
     CVEIgnoreList += CVEKnownList
 
 if opt.known:
@@ -1520,7 +1576,7 @@ if opt.rhel8oval:
     args.append(untriaged_json)
 
 debian_import_json = ""
-if opt.import_missing_debian and handler.debian is not None:
+if (opt.import_missing_debian or opt.mistriaged) and handler.debian is not None:
     debian_import_json = import_debian(handler)
     args.append(debian_import_json)
 
@@ -1630,6 +1686,26 @@ for cve in new_cves:
     if len(specific_cves) > 0 and cve not in specific_cves:
         # ignore this cve
         continue
+    # if this got marked as mistriaged, probablistically choose it for
+    # processing
+    if mistriaged_hint in handler.cve_data[cve]['desc']:
+      if opt.mistriaged == 0:
+          # ignore this one
+          continue
+      else:
+          # choose CVEs at random but pick most recent first - if year is
+          # now then we want to pick it, if year is 1 year ago we want to
+          # pick half as often, if year is 2 years ago we want to pick
+          # 1/4 as often etc - so do 1/2^(diff)
+          year = int(re.split('-', cve)[1])
+          now = datetime.datetime.utcnow().year
+          prob = 1.0 / math.pow(2, now - year)
+          rand = random.random()
+          if rand > prob:
+              continue
+          # selected!
+          opt.mistriaged = opt.mistriaged - 1
+
     count += 1
 
     if opt.report:
@@ -1667,7 +1743,7 @@ if experimental:
         if total > 0:
             _spawn_editor(fout.name)
         else:
-            prompt_user('Not spawning editor when as no CVEs to process')
+            prompt_user('Not spawning editor as no CVEs to process')
         fout.seek(0)
         try:
             handler.process_command_file(fout, preprocess=True)
diff --git a/scripts/process_cves b/scripts/process_cves
index 5bcb619..b4db9bf 100755
--- a/scripts/process_cves
+++ b/scripts/process_cves
@@ -126,6 +126,11 @@ process_missing_debian() {
     ./scripts/check-cves --import-missing-debian
 }
 
+mistriage() {
+    echo "check-cves --mistriage 10"
+    ./scripts/check-cves --mistriage 10
+}
+
 download_missing_rhsa() {
     tmpdir="$1"
     archive="$2.txt"
@@ -236,7 +241,8 @@ case "$action" in
         echo "Skipping merge from kernel team"
         echo "check-cves nvdcve-*.json"
         ./scripts/check-cves nvdcve-*.json
-        process_missing_debian
+        # this will also do missing debian
+        mistriage
 
         check_syntax
         nag_about_mailing_lists
@@ -270,6 +276,10 @@ case "$action" in
         update_debian
         process_missing_debian
         ;;
+    mistriage)
+        update_debian
+        mistriage
+        ;;
     update)
         # Do no triage
         update_uct