[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

improving the report-vuln script



Hi,

I had some spare time today and I figured I would look at the backlog of
unreported vulnerabilities here:

https://security-tracker.debian.org/tracker/status/unreported

This is one of the housekeeping tasks we were told was useful for the
security team here:

https://wiki.debian.org/LTS/Development#Housekeeping_Tasks

The instructions for using the report-vuln script weren't quite
accurate: first it can't really be used with reportbug, from what I can
tell, at least not in a useful way.. Reportbug will still prompt for all
its usual stuff which makes it less effective.

Furthermore, the recommended use (in the script itself) of Mutt doesn't
add the necessary headers, and I found the "blanks" argument rather
confusing.

Therefore, I have rewritten parts of the script to make it easier to
automate. It's now possible to specify the severity and affected
versions directly on the commandline, and the usage is much clearer.

Before:

$ ./bin/report-vuln -h
./bin/report-vuln [--no-blanks] <pkg> <cve id(s)>

Now:

usage: report-vuln [-h] [--no-blanks] [--affected AFFECTED]
                   [--severity SEVERITY] [--no-cc] [--cc-list CCLIST]
                   pkg cve [cve ...]

positional arguments:
  pkg                   affected package
  cve                   relevant CVE for this issue, may be used multiple time
                        if the issue has multiple CVEs

optional arguments:
  -h, --help            show this help message and exit
  --no-blanks, --blanks
                        include blank fields to be filled (default: False)
  --affected AFFECTED   affected version (default: unspecified)
  --severity SEVERITY   severity (default: grave)
  --no-cc, --cc         add X-Debbugs-CC header to
  --cc-list CCLIST      list of addres to add in CC (default:
                        ['team@security.debian.org', 'secure-testing-
                        team@lists.alioth.debian.org'])

I was not sure in which case multiple CVEs could be used - should we
report multiple CVEs in a single bug? that seems like bad
practice... IMHO, each bug should have its own CVE...

Ideally, this script would interactively submit the bug and wait for
confirmation, but it seems this is something that is notoriously hard to
do in the Debian BTS - I personnally haven't found a way to promptly get
a bug identifier when submitting bugs other than waiting for the
autoreply to kick in, which can take some time.

Since this is not something the LTS team directly uses all the time -
it's more something for the main security team - I figured it would be
safer to submit it for review here, especially since the changes are
rather large. Let me know if there's a better place to discuss this if
this isn't appropriate.

Attached is the patch for the new features and a fresh copy which is
only twice as long as the diff...

--- ../secure-testing/bin/report-vuln	2017-03-30 14:17:31.262515489 -0400
+++ report-vuln	2017-03-30 14:53:50.614772451 -0400
@@ -19,6 +19,7 @@
 #
 # export http_proxy if you need to use an http proxy to report bugs
 
+import argparse
 import sys, re, urllib, os
 
 temp_id = re.compile('(?:CVE|cve)\-[0-9]{4}-XXXX')
@@ -112,7 +113,7 @@
 
 	return ret + '\n'
 
-def gen_text(pkg, cveid, include_version = False, severity = 'FILLINSEVERITY'):
+def gen_text(pkg, cveid, blanks = False, severity = 'FILLINSEVERITY', affected=None, cc=False, cclist=None):
 	vuln_suff = 'y'
 	cve_suff = ''
 	time_w = 'was'
@@ -124,8 +125,13 @@
 		time_w = 'were'
 	
 	header = '''Package: %s\n''' % (pkg)
-	if include_version:
-		header += 'Version: FILLINAFFECTEDVERSION\n'
+	if affected is None:
+	        if blanks:
+		        header += "Version: FILLINAFFECTEDVERSION\n"
+        else:
+                header += "Version: %s\n" % affected
+        if cc and len(cclist) > 0:
+                header += "X-Debbugs-CC: %s\n" % " ".join(cclist)
 	header += '''Severity: %s
 Tags: security
 
@@ -160,31 +166,54 @@
 		print '\nhttps://security-tracker.debian.org/tracker/source-package/%s' % (pkg)
 		print '(issues without CVE id are assigned a TEMP one, but it may change over time)\n'
 
-	if not include_version:
-		print '''Please adjust the affected versions in the BTS as needed.\n'''
+	if not blanks:
+		print '''\nPlease adjust the affected versions in the BTS as needed.\n'''
 
 def error(msg):
 	print 'error: ' + msg
 	sys.exit(1)
 
-def usage():
-	print sys.argv[0], '[--no-blanks] <pkg> <cve id(s)>'
-	sys.exit(0)
+class NegateAction(argparse.Action):
+    '''add a toggle flag to argparse
+
+    this is similar to 'store_true' or 'store_false', but allows
+    arguments prefixed with --no to disable the default. the default
+    is set depending on the first argument - if it starts with the
+    negative form (define by default as '--no'), the default is False,
+    otherwise True.
+    '''
+
+    negative = '--no'
+
+    def __init__(self, option_strings, *args, **kwargs):
+        '''set default depending on the first argument'''
+        default = not option_strings[0].startswith(self.negative)
+        super(NegateAction, self).__init__(option_strings, *args,
+                                           default=default, nargs=0, **kwargs)
+
+    def __call__(self, parser, ns, values, option):
+        '''set the truth value depending on whether
+        it starts with the negative form'''
+        setattr(ns, self.dest, not option.startswith(self.negative))
 
-def main():
-	if len(sys.argv) < 3:
-		usage()
 
-	blanks = True
-	if sys.argv[1] == '--no-blanks':
-		if len(sys.argv) < 4:
-			usage()
-		blanks = False
-		pkg = sys.argv[2]
-		cve = sys.argv[3:]
-	else:
-		pkg = sys.argv[1]
-		cve = sys.argv[2:]
+def main():
+        parser = argparse.ArgumentParser()
+        parser.add_argument('--no-blanks', '--blanks', dest='blanks', action=NegateAction,
+                            help='include blank fields to be filled (default: %(default)s)')
+        parser.add_argument('--affected', help='affected version (default: unspecified)')
+        parser.add_argument('--severity', default='grave', help='severity (default: %(default)s)')
+        parser.add_argument('--no-cc', '--cc', dest='cc', action=NegateAction,
+                            help='add X-Debbugs-CC header to')
+        parser.add_argument('--cc-list', dest='cclist', default=['team@security.debian.org', 'secure-testing-team@lists.alioth.debian.org'],
+                            help='list of addres to add in CC (default: %(default)s)')
+        parser.add_argument('pkg', help='affected package')
+        parser.add_argument('cve', nargs='+', help='relevant CVE for this issue, may be used multiple time if the issue has multiple CVEs')
+        args = parser.parse_args()
+
+        blanks = args.blanks
+        pkg = args.pkg
+        cve = args.cve
 
 	# check for valid parameters
 	p = re.compile('^[0-9a-z].*')
@@ -197,10 +226,7 @@
 		if not c.match(arg) and not temp_id.match(arg):
 			error(arg + ' does not seem to be a valid CVE id')
 
-	if blanks:
-		gen_text(pkg, cve)
-	else:
-		gen_text(pkg, cve, False, 'grave')
+	gen_text(pkg, cve, affected=args.affected, blanks=args.blanks, severity=args.severity, cc=args.cc, cclist=args.cclist)
 
 if __name__ == '__main__':
 	main()

Attachment: report-vuln
Description: python

A.

-- 
Worker bees can leave
Even drones can fly away
The queeen is their slave
                        - Fight Club

Reply to: