New upstream version 2.20.4

NEWS

@@ -1,6 +1,54 @@
 This file summarizes the major and interesting changes for each release. For a
 detailed list of changes, please see ChangeLog.
 
+2.20.4 (2016-12-14)
+-------------------
+ * SECURITY FIX: Restrict a report's CrashDB field to literals.
+   Use ast.literal_eval() instead of the generic eval(), to prevent arbitrary
+   code execution from malicious .crash files. A user could be tricked into
+   opening a crash file whose CrashDB field contains an exec(), open(), or
+   similar commands; this is fairly easy as we install a MIME handler for
+   these. Thanks to Donncha O'Cearbhaill for discovering this!
+   (CVE-2016-9949, LP: #1648806)
+ * SECURITY FIX: Fix path traversal vulnerability with hooks execution.
+   Ensure that Package: and SourcePackage: fields loaded from reports do not
+   contain directories. Until now, an attacker could trick a user into opening a
+   malicious .crash file containing
+
+     Package: ../../../../some/dir/foo
+
+   which would execute /some/dir/foo.py with arbitrary code.
+   Thanks to Donncha O'Cearbhaill for discovering this!
+   (CVE-2016-9950, LP: #1648806)
+ * SECURITY FIX: apport-{gtk,kde}: Only offer "Relaunch" for recent /var/crash
+   crashes.
+   It only makes sense to offer relaunching for crashes that just happened and
+   the apport UI got triggered on those. When opening a .crash file copied from
+   somewhere else or after the crash happened, this is even actively dangerous
+   as a malicious crash file can specify any arbitrary command to run.
+   Thanks to Donncha O'Cearbhaill for discovering this!
+   (CVE-2016-9951, LP: #1648806)
+ * test_backend_apt_dpkg.py: Move tests from Ubuntu 15.10 "wily" (which is EOL
+   now) to 16.04 LTS "xenial".
+ * packaging-apt-dpkg.py: Explicitly set Dir::State::Status to the host
+   dpkg status file for get_source_tree(), to work with apt 1.3~pre4.
+ * packaging-apt-dpkg.py: Change the proxy settings to use "DIRECT" instead
+   of "direct". The latter never really worked, but APT did not complain about
+   it.
+ * data/iwlwifi_error_dump: Fix add_package() call.
+ * hookutils.py, attach_mac_events(): Only attach /proc/version_signature if
+   that actually exists.
+ * test/test_report.py: Slightly relax stack trace checks to also work with
+   glibc 2.24.
+ * apport-gtk: Specify module version with GI imports to avoid warnings. Thanks
+   Anatoly Techtonik. (LP: #1502173)
+ * test/run: Prefer pycodestyle over pep8.
+ * backends/packaging-apt-dpkg.py: provide a fallback method if using zgrep to
+   search for a file in Contents.gz fails due to a lack of memory.  Thanks
+   Brian Murray.
+ * bin/apport-retrace: When --core-file is used instead of loading the core
+   file and adding it to the apport report just pass the file reference to gdb.
+
 2.20.3 (2016-07-28)
 -------------------
  * problem_report.py: Fail with proper exception when trying to assign a list

apport/crashdb_impl/launchpad.py

@@ -997,6 +997,7 @@ def _filter_tag_names(klass, tags):
 
         return res
 
+
 #
 # Launchpad storeblob API (should go into launchpadlib, see LP #315358)
 #
@@ -1095,6 +1096,7 @@ def upload_blob(blob, progress_callback=None, hostname='launchpad.net'):
     assert ticket
     return ticket
 
+
 #
 # Unit tests
 #

apport/fileutils.py

@@ -338,6 +338,7 @@ def get_config(section, setting, default=None, path=None, bool=False):
     except (NoOptionError, NoSectionError):
         return default
 
+
 get_config.config = None
 
 

apport/hookutils.py

@@ -57,7 +57,7 @@ def attach_file_if_exists(report, path, key=None, overwrite=True, force_unicode=
     new key with '_' appended will be added instead.
 
     If the contents is valid UTF-8, or force_unicode is True, then the value
-    will a string, otherwise it will be bytes.
+    will be a string, otherwise it will be bytes.
     '''
     if not key:
         key = path_to_key(path)
@@ -253,14 +253,14 @@ def attach_hardware(report):
     report['UdevDb'] = command_output(['udevadm', 'info', '--export-db'])
 
     # anonymize partition labels
-    l = report['UdevDb']
-    l = re.sub('ID_FS_LABEL=(.*)', 'ID_FS_LABEL=<hidden>', l)
-    l = re.sub('ID_FS_LABEL_ENC=(.*)', 'ID_FS_LABEL_ENC=<hidden>', l)
-    l = re.sub('by-label/(.*)', 'by-label/<hidden>', l)
-    l = re.sub('ID_FS_LABEL=(.*)', 'ID_FS_LABEL=<hidden>', l)
-    l = re.sub('ID_FS_LABEL_ENC=(.*)', 'ID_FS_LABEL_ENC=<hidden>', l)
-    l = re.sub('by-label/(.*)', 'by-label/<hidden>', l)
-    report['UdevDb'] = l
+    labels = report['UdevDb']
+    labels = re.sub('ID_FS_LABEL=(.*)', 'ID_FS_LABEL=<hidden>', labels)
+    labels = re.sub('ID_FS_LABEL_ENC=(.*)', 'ID_FS_LABEL_ENC=<hidden>', labels)
+    labels = re.sub('by-label/(.*)', 'by-label/<hidden>', labels)
+    labels = re.sub('ID_FS_LABEL=(.*)', 'ID_FS_LABEL=<hidden>', labels)
+    labels = re.sub('ID_FS_LABEL_ENC=(.*)', 'ID_FS_LABEL_ENC=<hidden>', labels)
+    labels = re.sub('by-label/(.*)', 'by-label/<hidden>', labels)
+    report['UdevDb'] = labels
 
     attach_dmi(report)
 
@@ -360,7 +360,7 @@ def command_available(command):
 
 def command_output(command, input=None, stderr=subprocess.STDOUT,
                    keep_locale=False, decode_utf8=True):
-    '''Try to execute given command (array) and return its stdout.
+    '''Try to execute given command (list) and return its stdout.
 
     In case of failure, a textual error gets returned. This function forces
     LC_MESSAGES to C, to avoid translated output in bug reports.
@@ -399,7 +399,7 @@ def _root_command_prefix():
 
 
 def root_command_output(command, input=None, stderr=subprocess.STDOUT, decode_utf8=True):
-    '''Try to execute given command (array) as root and return its stdout.
+    '''Try to execute given command (list) as root and return its stdout.
 
     This passes the command through pkexec, unless the caller is already root.
 
@@ -524,6 +524,7 @@ def xsession_errors(pattern=None):
                 lines += line
     return lines
 
+
 PCI_MASS_STORAGE = 0x01
 PCI_NETWORK = 0x02
 PCI_DISPLAY = 0x03
@@ -715,7 +716,7 @@ def attach_mac_events(report, profiles=None):
     if 'AuditLog' not in report and os.path.exists('/var/run/auditd.pid'):
         attach_root_command_outputs(report, {'AuditLog': 'egrep "' + mac_regex + '" /var/log/audit/audit.log'})
 
-    attach_file(report, '/proc/version_signature', 'ProcVersionSignature')
+    attach_file_if_exists(report, '/proc/version_signature', 'ProcVersionSignature')
     attach_file(report, '/proc/cmdline', 'ProcCmdline')
 
     for match in re.findall(aa_re, report.get('KernLog', '') + report.get('AuditLog', '')):
@@ -818,8 +819,8 @@ def nonfree_kernel_modules(module_list='/proc/modules'):
 
     nonfree = []
     for m in mods:
-        l = _get_module_license(m)
-        if l and not ('GPL' in l or 'BSD' in l or 'MPL' in l or 'MIT' in l):
+        s = _get_module_license(m)
+        if s and not ('GPL' in s or 'BSD' in s or 'MPL' in s or 'MIT' in s):
             nonfree.append(m)
 
     return nonfree
@@ -869,10 +870,10 @@ def in_session_of_problem(report):
     if not session_id:
         # fall back to reading cgroup
         with open('/proc/self/cgroup') as f:
-            for l in f:
-                l = l.strip()
-                if 'name=systemd:' in l and l.endswith('.scope') and '/session-' in l:
-                    session_id = l.split('/session-', 1)[1][:-6]
+            for line in f:
+                line = line.strip()
+                if 'name=systemd:' in line and line.endswith('.scope') and '/session-' in line:
+                    session_id = line.split('/session-', 1)[1][:-6]
                     break
             else:
                 return None

apport/report.py

@@ -381,9 +381,9 @@ def _check_interpreted(self):
 
         # first, determine process name
         name = None
-        for l in self['ProcStatus'].splitlines():
+        for line in self['ProcStatus'].splitlines():
             try:
-                (k, v) = l.split('\t', 1)
+                (k, v) = line.split('\t', 1)
             except ValueError:
                 continue
             if k == 'Name:':
@@ -590,13 +590,13 @@ def add_proc_environ(self, pid=None, extraenv=[]):
         if env.startswith('Error:'):
             self['ProcEnviron'] = env
         else:
-            for l in env.split('\0'):
-                if l.split('=', 1)[0] in safe_vars:
+            for line in env.split('\0'):
+                if line.split('=', 1)[0] in safe_vars:
                     if self['ProcEnviron']:
                         self['ProcEnviron'] += '\n'
-                    self['ProcEnviron'] += l
-                elif l.startswith('PATH='):
-                    p = l.split('=', 1)[1]
+                    self['ProcEnviron'] += line
+                elif line.startswith('PATH='):
+                    p = line.split('=', 1)[1]
                     if '/home' in p or '/tmp' in p:
                         if self['ProcEnviron']:
                             self['ProcEnviron'] += '\n'
@@ -605,20 +605,20 @@ def add_proc_environ(self, pid=None, extraenv=[]):
                         if self['ProcEnviron']:
                             self['ProcEnviron'] += '\n'
                         self['ProcEnviron'] += 'PATH=(custom, no user)'
-                elif l.startswith('XDG_RUNTIME_DIR='):
+                elif line.startswith('XDG_RUNTIME_DIR='):
                     if self['ProcEnviron']:
                         self['ProcEnviron'] += '\n'
                     self['ProcEnviron'] += 'XDG_RUNTIME_DIR=<set>'
-                elif l.startswith('LD_PRELOAD='):
+                elif line.startswith('LD_PRELOAD='):
                     if self['ProcEnviron']:
                         self['ProcEnviron'] += '\n'
                     self['ProcEnviron'] += 'LD_PRELOAD=<set>'
-                elif l.startswith('LD_LIBRARY_PATH='):
+                elif line.startswith('LD_LIBRARY_PATH='):
                     if self['ProcEnviron']:
                         self['ProcEnviron'] += '\n'
                     self['ProcEnviron'] += 'LD_LIBRARY_PATH=<set>'
-                elif l.startswith('XDG_CURRENT_DESKTOP='):
-                    self['CurrentDesktop'] = l.split('=', 1)[1]
+                elif line.startswith('XDG_CURRENT_DESKTOP='):
+                    self['CurrentDesktop'] = line.split('=', 1)[1]
 
     def add_kernel_crash_info(self, debugdir=None):
         '''Add information from kernel crash.
@@ -828,14 +828,21 @@ def add_hooks_info(self, ui, package=None, srcpackage=None):
         False otherwise.
         '''
         # determine package names, unless already given as arguments
+        # avoid path traversal
         if not package:
             package = self.get('Package')
         if package:
             package = package.split()[0]
+            if '/' in package:
+                self['UnreportableReason'] = 'invalid Package: %s' % package
+                return
         if not srcpackage:
             srcpackage = self.get('SourcePackage')
         if srcpackage:
             srcpackage = srcpackage.split()[0]
+            if '/' in srcpackage:
+                self['UnreportableReason'] = 'invalid SourcePackage: %s' % package
+                return
 
         hook_dirs = [_hook_dir]
         # also search hooks in /opt, when program is from there
@@ -1082,8 +1089,8 @@ def has_useful_stacktrace(self):
     def stacktrace_top_function(self):
         '''Return topmost function in StacktraceTop'''
 
-        for l in self.get('StacktraceTop', '').splitlines():
-            fname = l.split('(')[0].strip()
+        for line in self.get('StacktraceTop', '').splitlines():
+            fname = line.split('(')[0].strip()
             if fname != '??':
                 return fname
 
@@ -1224,10 +1231,10 @@ def obsolete_packages(self):
         '''Return list of obsolete packages in Package and Dependencies.'''
 
         obsolete = []
-        for l in (self.get('Package', '') + '\n' + self.get('Dependencies', '')).splitlines():
-            if not l:
+        for line in (self.get('Package', '') + '\n' + self.get('Dependencies', '')).splitlines():
+            if not line:
                 continue
-            pkg, ver = l.split()[:2]
+            pkg, ver = line.split()[:2]
             avail = packaging.get_available_version(pkg)
             if ver is not None and ver != 'None' and avail is not None and packaging.compare_versions(ver, avail) < 0:
                 obsolete.append(pkg)
@@ -1309,8 +1316,8 @@ def crash_signature(self):
                 return None
 
             loc_re = re.compile('^\s+File "([^"]+).*line (\d+).*\sin (.*)$')
-            for l in trace:
-                m = loc_re.match(l)
+            for line in trace:
+                m = loc_re.match(line)
                 if m:
                     # if we have a function name, use this; for a a crash
                     # outside of a function/method, fall back to the source
@@ -1606,10 +1613,10 @@ def get_logind_session(klass, pid):
         # determine cgroup
         try:
             with open('/proc/%s/cgroup' % pid) as f:
-                for l in f:
-                    l = l.strip()
-                    if 'name=systemd:' in l and l.endswith('.scope') and '/session-' in l:
-                        my_session = l.split('/session-', 1)[1][:-6]
+                for line in f:
+                    line = line.strip()
+                    if 'name=systemd:' in line and line.endswith('.scope') and '/session-' in line:
+                        my_session = line.split('/session-', 1)[1][:-6]
                         break
                 else:
                     return None

apport/ui.py

@@ -18,6 +18,7 @@
 import subprocess, threading, webbrowser
 import signal
 import time
+import ast
 
 import apport, apport.fileutils, apport.REThread
 
@@ -32,7 +33,7 @@
     from configparser import ConfigParser
     PY3 = True
 
-__version__ = '2.20.3'
+__version__ = '2.20.4'
 
 
 def excstr(exception):
@@ -42,6 +43,7 @@ def excstr(exception):
         return str(exception).decode(locale.getpreferredencoding(), 'replace')
     return str(exception)
 
+
 symptom_script_dir = os.environ.get('APPORT_SYMPTOMS_DIR',
                                     '/usr/share/apport/symptoms')
 PF_KTHREAD = 0x200000
@@ -187,6 +189,7 @@ def __init__(self):
         self.report = None
         self.report_file = None
         self.cur_package = None
+        self.offer_restart = False
 
         try:
             self.crashdb = apport.crashdb.get_crashdb(None)
@@ -217,6 +220,9 @@ def run_crashes(self):
         otherwise.
         '''
         result = False
+        # for iterating over /var/crash (as opposed to running on or clicking
+        # on a particular .crash file) we offer restarting
+        self.offer_restart = True
 
         if os.geteuid() == 0:
             reports = apport.fileutils.get_new_system_reports()
@@ -919,11 +925,11 @@ def check_report_crashdb(self):
         # specification?
         if self.report['CrashDB'].lstrip().startswith('{'):
             try:
-                spec = eval(self.report['CrashDB'], {})
+                spec = ast.literal_eval(self.report['CrashDB'])
                 assert isinstance(spec, dict)
                 assert 'impl' in spec
-            except:
-                self.report['UnreportableReason'] = 'A package hook defines an invalid crash database definition:\n%s' % self.report['CrashDB']
+            except Exception as e:
+                self.report['UnreportableReason'] = 'A package hook defines an invalid crash database definition:\n%s\n%s' % (self.report['CrashDB'], e)
                 return False
             try:
                 self.crashdb = apport.crashdb.load_crashdb(None, spec)