diff -Nru apport-2.20.11/apport/fileutils.py apport-2.20.11/apport/fileutils.py --- apport-2.20.11/apport/fileutils.py 2021-03-24 14:58:15.000000000 +0000 +++ apport-2.20.11/apport/fileutils.py 2021-10-18 11:48:31.000000000 +0000 @@ -12,6 +12,7 @@ import os, glob, subprocess, os.path, time, pwd, sys, stat, json, socket import http.client from contextlib import closing +from operator import itemgetter try: from configparser import (ConfigParser, NoOptionError, NoSectionError, @@ -27,6 +28,8 @@ from apport.packaging_impl import impl as packaging report_dir = os.environ.get('APPORT_REPORT_DIR', '/var/crash') +core_dir = '/var/lib/apport/coredump' +max_corefiles_per_uid = 5 _config_file = '~/.config/apport/settings' @@ -167,7 +170,7 @@ else: raise ValueError('report does not have the ExecutablePath attribute') - uid = os.getuid() + uid = os.geteuid() base = '%s.%s.%s.hanging' % (subject, str(uid), pid) path = os.path.join(report_dir, base) with open(path, 'a'): @@ -310,7 +313,7 @@ def make_report_file(report, uid=None): '''Construct a canonical pathname for a report and open it for writing - If uid is not given, it defaults to the uid of the current process. + If uid is not given, it defaults to the effective uid of the current process. The report file must not exist already, to prevent losing previous reports or symlink attacks. @@ -324,7 +327,7 @@ raise ValueError('report has neither ExecutablePath nor Package attribute') if not uid: - uid = os.getuid() + uid = os.geteuid() path = os.path.join(report_dir, '%s.%s.crash' % (subject, str(uid))) if sys.version >= '3': @@ -363,21 +366,22 @@ This is read from ~/.config/apport/settings or path. If bool is True, the value is interpreted as a boolean. + + Privileges may need to be dropped before calling this. ''' + if not path: - path = os.path.expanduser(_config_file) + # Properly handle dropped privileges + homedir = pwd.getpwuid(os.geteuid())[5] + path = _config_file.replace("~", homedir) contents = '' fd = None f = None if not get_config.config: get_config.config = ConfigParser() - egid = os.getegid() - euid = os.geteuid() + try: - # drop permissions temporarily to try open users config file - os.setegid(os.getgid()) - os.seteuid(os.getuid()) fd = os.open(path, os.O_NOFOLLOW | os.O_RDONLY) st = os.fstat(fd) if stat.S_ISREG(st.st_mode): @@ -391,8 +395,6 @@ f.close() elif fd is not None: os.close(fd) - os.seteuid(euid) - os.setegid(egid) try: get_config.config.read_string(contents) @@ -439,6 +441,78 @@ return (real_uid, real_gid) +def get_boot_id(): + '''Gets the kernel boot id''' + + with open('/proc/sys/kernel/random/boot_id') as f: + boot_id = f.read().strip() + return boot_id + + +def get_core_path(pid=None, exe=None, uid=None, timestamp=None): + '''Get the path to a core file''' + + if pid is None: + pid = 'unknown' + timestamp = 'unknown' + else: + if timestamp is None: + with open("/proc/%s/stat" % pid) as stat_file: + stat_contents = stat_file.read() + timestamp = get_starttime(stat_contents) + + if exe is None: + exe = 'unknown' + else: + exe = exe.replace('/', '_').replace('.', '_') + + if uid is None: + uid = os.getuid() + + # This is similar to systemd-coredump, but with the exe name instead + # of the command name + core_name = ('core.%s.%s.%s.%s.%s' % (exe, uid, get_boot_id(), + str(pid), str(timestamp))) + + core_path = os.path.join(core_dir, core_name) + + return (core_name, core_path) + + +def find_core_files_by_uid(uid): + '''Searches the core file directory for files that belong to a + specified uid. Returns a list of lists containing the filename and + the file modification time.''' + core_files = [] + uid_files = [] + + if os.path.exists(core_dir): + core_files = os.listdir(path=core_dir) + + for f in core_files: + try: + if f.split('.')[2] == str(uid): + time = os.path.getmtime(os.path.join(core_dir, f)) + uid_files.append([f, time]) + except (IndexError, FileNotFoundError): + continue + return uid_files + + +def clean_core_directory(uid): + '''Removes old files from the core directory if there are more than + the maximum allowed per uid''' + + uid_files = find_core_files_by_uid(uid) + sorted_files = sorted(uid_files, key=itemgetter(1)) + + # Substract a extra one to make room for the new core file + if len(uid_files) > max_corefiles_per_uid - 1: + for x in range(len(uid_files) - max_corefiles_per_uid + 1): + os.remove(os.path.join(core_dir, sorted_files[0][0])) + sorted_files.remove(sorted_files[0]) + + def shared_libraries(path): '''Get libraries with which the specified binary is linked. diff -Nru apport-2.20.11/apport/report.py apport-2.20.11/apport/report.py --- apport-2.20.11/apport/report.py 2021-04-07 15:27:44.000000000 +0000 +++ apport-2.20.11/apport/report.py 2021-10-14 15:05:22.000000000 +0000 @@ -422,7 +422,8 @@ This adds: - UserGroups: system groups the user is in ''' - user = pwd.getpwuid(os.getuid())[0] + # Use effective uid in case privileges were dropped + user = pwd.getpwuid(os.geteuid())[0] groups = [name for name, p, gid, memb in grp.getgrall() if user in memb and gid < 1000] groups.sort() @@ -1044,21 +1045,15 @@ Raises ValueError if the file exists but is invalid XML. ''' - orig_home = os.getenv('HOME') - if orig_home is not None: - del os.environ['HOME'] - ifpath = os.path.expanduser(_ignore_file) - if orig_home is not None: - os.environ['HOME'] = orig_home + # Properly handle dropped privileges + homedir = pwd.getpwuid(os.geteuid())[5] + ifpath = _ignore_file.replace("~", homedir) + contents = '' fd = None f = None - egid = os.getegid() - euid = os.geteuid() + try: - # drop permissions temporarily to try open users ignore file - os.setegid(os.getgid()) - os.seteuid(os.getuid()) fd = os.open(ifpath, os.O_NOFOLLOW | os.O_RDONLY) st = os.fstat(fd) if stat.S_ISREG(st.st_mode): @@ -1072,8 +1067,6 @@ f.close() elif fd is not None: os.close(fd) - os.seteuid(euid) - os.setegid(egid) if contents == '': # create a document from scratch @@ -1102,6 +1095,8 @@ This requires the ExecutablePath attribute. Throws a ValueError if the file has an invalid format. + + Privileges may need to be dropped before calling this. ''' assert 'ExecutablePath' in self @@ -1167,6 +1162,8 @@ Throws a ValueError if the file already exists and has an invalid format. + + Privileges may need to be dropped before calling this. ''' assert 'ExecutablePath' in self @@ -1192,14 +1189,10 @@ e.setAttribute('mtime', mtime) dom.documentElement.appendChild(e) - # write back file; temporarily unset $HOME, as this gets the wrong home - # dir for e. g. sudo - orig_home = os.getenv('HOME') - if orig_home is not None: - del os.environ['HOME'] - ignore_file_path = os.path.expanduser(_ignore_file) - if orig_home is not None: - os.environ['HOME'] = orig_home + # Write back file + # Properly handle dropped privileges + homedir = pwd.getpwuid(os.geteuid())[5] + ignore_file_path = _ignore_file.replace("~", homedir) with open(ignore_file_path, 'w') as fd: dom.writexml(fd, addindent=' ', newl='\n') @@ -1596,9 +1589,10 @@ removes the ProcCwd attribute completely. ''' replacements = [] + # Do not replace "root" if (os.getuid() > 0): - # do not replace "root" - p = pwd.getpwuid(os.getuid()) + # Use effective uid in case privileges were dropped + p = pwd.getpwuid(os.geteuid()) if len(p[0]) >= 2: replacements.append((re.compile(r'\b%s\b' % re.escape(p[0])), 'username')) replacements.append((re.compile(r'\b%s\b' % re.escape(p[5])), '/home/username')) diff -Nru apport-2.20.11/data/apport apport-2.20.11/data/apport --- apport-2.20.11/data/apport 2021-03-24 14:58:15.000000000 +0000 +++ apport-2.20.11/data/apport 2021-10-18 11:45:26.000000000 +0000 @@ -55,7 +55,7 @@ signal.signal(signal.SIGALRM, original_handler) -(pidstat, real_uid, real_gid, cwd, proc_pid_fd) = (None, None, None, None, None) +(pidstat, crash_uid, crash_gid, cwd, proc_pid_fd) = (None, None, None, None, None) def proc_pid_opener(path, flags): @@ -65,7 +65,7 @@ def get_pid_info(pid): '''Read /proc information about pid''' - global pidstat, real_uid, real_gid, cwd, proc_pid_fd + global pidstat, crash_uid, crash_gid, cwd, proc_pid_fd proc_pid_fd = os.open('/proc/%s' % pid, os.O_RDONLY | os.O_PATH | os.O_DIRECTORY) @@ -73,15 +73,15 @@ # here -- we want to know in the log file pidstat = os.stat('stat', dir_fd=proc_pid_fd) - # determine real UID of the target process; do *not* use the owner of + # determine UID and GID of the target process; do *not* use the owner of # /proc/pid/stat, as that will be root for setuid or unreadable programs! # (this matters when suid_dumpable is enabled) with open('status', opener=proc_pid_opener) as status_file: contents = status_file.read() - (real_uid, real_gid) = apport.fileutils.get_uid_and_gid(contents) + (crash_uid, crash_gid) = apport.fileutils.get_uid_and_gid(contents) - assert real_uid is not None, 'failed to parse Uid' - assert real_gid is not None, 'failed to parse Gid' + assert crash_uid is not None, 'failed to parse Uid' + assert crash_gid is not None, 'failed to parse Gid' cwd = os.open('cwd', os.O_RDONLY | os.O_PATH | os.O_DIRECTORY, dir_fd=proc_pid_fd) @@ -102,26 +102,26 @@ return apport.fileutils.get_starttime(contents) -def drop_privileges(real_only=False): - '''Change user and group to real_[ug]id +def drop_privileges(): + '''Change effective user and group to crash_[ug]id + ''' + # Drop any supplemental groups, or we'll still be in the root group + if os.getuid() == 0: + os.setgroups([]) + assert os.getgroups() == [] + os.setregid(-1, crash_gid) + os.setreuid(-1, crash_uid) + assert os.getegid() == crash_gid + assert os.geteuid() == crash_uid + - Normally that irrevocably drops privileges to the real user/group of the - target process. With real_only=True only the real IDs are changed, but - the effective IDs remain. +def recover_privileges(): + '''Change effective user and group back to real uid and gid ''' - if real_only: - # Drop any supplemental groups - if os.getuid() == 0: - os.setgroups([]) - os.setregid(real_gid, -1) - os.setreuid(real_uid, -1) - else: - os.setgid(real_gid) - os.setuid(real_uid) - assert os.getegid() == real_gid - assert os.geteuid() == real_uid - assert os.getgid() == real_gid - assert os.getuid() == real_uid + os.setregid(-1, os.getgid()) + os.setreuid(-1, os.getuid()) + assert os.getegid() == os.getgid() + assert os.geteuid() == os.getuid() def init_error_log(): @@ -179,8 +179,8 @@ signal.signal(signal.SIGBUS, _log_signal_handler) -def write_user_coredump(pid, cwd, limit, from_report=None): - '''Write the core into the current directory if ulimit requests it.''' +def write_user_coredump(pid, timestamp, limit, from_report=None): + '''Write the core into a directory if ulimit requests it.''' # three cases: # limit == 0: do not write anything @@ -194,23 +194,25 @@ # protected executables, in accordance with core(5) # (suid_dumpable==2 and core_pattern restrictions); when this happens, # /proc/pid/stat is owned by root (or the user suid'ed to), but we already - # changed to the crashed process' real uid + # changed to the crashed process' uid assert pidstat, 'pidstat not initialized' - if pidstat.st_uid != os.getuid() or pidstat.st_gid != os.getgid(): + if pidstat.st_uid != crash_uid or pidstat.st_gid != crash_gid: error_log('disabling core dump for suid/sgid/unreadable executable') return + (core_name, core_path) = apport.fileutils.get_core_path(pid, + options.exe_path, + crash_uid, + timestamp) + try: - with open('/proc/sys/kernel/core_uses_pid') as f: - if f.read().strip() == '0': - core_path = 'core' - else: - core_path = 'core.%s' % (str(pid)) - core_file = os.open(core_path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, mode=0o600, dir_fd=cwd) + # Limit number of core files to prevent DoS + apport.fileutils.clean_core_directory(crash_uid) + core_file = os.open(core_path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, mode=0o400, dir_fd=cwd) except (OSError, IOError): return - error_log('writing core dump to %s (limit: %s)' % (core_path, str(limit))) + error_log('writing core dump to %s (limit: %s)' % (core_name, str(limit))) written = 0 @@ -224,7 +226,7 @@ os.close(core_file) os.unlink(core_path, dir_fd=cwd) return - error_log('writing core dump %s of size %i' % (core_path, core_size)) + error_log('writing core dump %s of size %i' % (core_name, core_size)) os.write(core_file, r['CoreDump']) else: # read from stdin @@ -247,6 +249,8 @@ return block = os.read(0, 1048576) + # Make sure the user can read it + os.fchown(core_file, crash_uid, -1) os.close(core_file) @@ -266,7 +270,7 @@ return (memfree + cached - writeback) * 1024 -def is_closing_session(uid): +def is_closing_session(): '''Check if pid is in a closing user session. During that, crashes are common as the session D-BUS and X.org are going @@ -282,11 +286,18 @@ error_log('is_closing_session(): no DBUS_SESSION_BUS_ADDRESS in environment') return False - euid = os.geteuid() - egid = os.getegid() + # We need to drop both the real and effective uid/gid before calling + # gdbus because DBUS_SESSION_BUS_ADDRESS is untrusted and may allow + # reading arbitrary files as a noncefile. We can't just drop effective + # uid/gid as gdbus has a check to make sure it's not running in a + # setuid environment and it does so by comparing the real and effective + # ids. We don't need to drop supplemental groups here, as the privilege + # dropping code elsewhere has already done so. + real_uid = os.getuid() + real_gid = os.getgid() try: - os.setegid(os.getgid()) - os.seteuid(os.getuid()) + os.setresgid(crash_gid, crash_gid, real_gid) + os.setresuid(crash_uid, crash_uid, real_uid) gdbus = subprocess.Popen(['/usr/bin/gdbus', 'call', '-e', '-d', 'org.gnome.SessionManager', '-o', '/org/gnome/SessionManager', '-m', 'org.gnome.SessionManager.IsSessionRunning'], stdout=subprocess.PIPE, @@ -298,8 +309,9 @@ error_log('gdbus call failed, cannot determine running session: ' + str(e)) return False finally: - os.seteuid(euid) - os.setegid(egid) + os.setresuid(real_uid, real_uid, -1) + os.setresgid(real_gid, real_gid, -1) + error_log('debug: session gdbus call: ' + out.decode('UTF-8')) if out.startswith(b'(false,'): return True @@ -597,9 +609,6 @@ error_log('process was replaced after Apport started, ignoring') sys.exit(0) - # Partially drop privs to gain proper os.access() checks - drop_privileges(True) - error_log('called for pid %s, signal %s, core limit %s, dump mode %s' % (pid, signum, core_ulimit, dump_mode)) try: @@ -620,15 +629,14 @@ # ignore SIGQUIT (it's usually deliberately generated by users) if signum == str(int(signal.SIGQUIT)): - drop_privileges() - write_user_coredump(pid, cwd, core_ulimit) + write_user_coredump(pid, process_start, core_ulimit) sys.exit(0) # check if the executable was modified after the process started (e. g. # package got upgraded in between) exe_mtime = os.stat('exe', dir_fd=proc_pid_fd).st_mtime - process_start = os.lstat('cmdline', dir_fd=proc_pid_fd).st_mtime - if not os.path.exists(os.readlink('exe', dir_fd=proc_pid_fd)) or exe_mtime > process_start: + process_mtime = os.lstat('cmdline', dir_fd=proc_pid_fd).st_mtime + if not os.path.exists(os.readlink('exe', dir_fd=proc_pid_fd)) or exe_mtime > process_mtime: error_log('executable was modified after program start, ignoring') sys.exit(0) @@ -647,18 +655,12 @@ if options.exe_path is not None and os.path.exists(options.exe_path): info['ExecutablePath'] = options.exe_path - euid = os.geteuid() - egid = os.getegid() - try: - # Drop permissions temporarily to make sure that we don't - # include information in the crash report that the user should - # not be allowed to access. - os.setegid(os.getgid()) - os.seteuid(os.getuid()) - info.add_proc_info(proc_pid_fd=proc_pid_fd) - finally: - os.seteuid(euid) - os.setegid(egid) + # Drop privileges temporarily to make sure that we don't + # include information in the crash report that the user should + # not be allowed to access. + drop_privileges() + + info.add_proc_info(proc_pid_fd=proc_pid_fd) if 'ExecutablePath' not in info: error_log('could not determine ExecutablePath, aborting') @@ -686,16 +688,16 @@ if not apport.fileutils.get_config('main', 'unpackaged', False, bool=True): error_log('executable does not belong to a package, ignoring') # check if the user wants a core dump - drop_privileges() - write_user_coredump(pid, cwd, core_ulimit) + recover_privileges() + write_user_coredump(pid, process_start, core_ulimit) sys.exit(0) # ignore SIGXCPU and SIGXFSZ since this indicates some external # influence changing soft RLIMIT values when running programs. if signum in [str(signal.SIGXCPU), str(signal.SIGXFSZ)]: error_log('Ignoring signal %s (caused by exceeding soft RLIMIT)' % signum) - drop_privileges() - write_user_coredump(pid, cwd, core_ulimit) + recover_privileges() + write_user_coredump(pid, process_start, core_ulimit) sys.exit(0) # ignore blacklisted binaries @@ -703,7 +705,11 @@ error_log('executable version is blacklisted, ignoring') sys.exit(0) - if is_closing_session(pidstat.st_uid): + # We can now recover privileges to create the crash report file and + # write out the user coredumps + recover_privileges() + + if is_closing_session(): error_log('happens for shutting down session, ignoring') sys.exit(0) @@ -730,8 +736,7 @@ crash_counter = apport.fileutils.get_recent_crashes(f) crash_counter += 1 if crash_counter > 1: - drop_privileges() - write_user_coredump(pid, cwd, core_ulimit) + write_user_coredump(pid, process_start, core_ulimit) error_log('this executable already crashed %i times, ignoring' % crash_counter) sys.exit(0) # remove the old file, so that we can create the new one with @@ -739,17 +744,10 @@ os.unlink(report) else: error_log('apport: report %s already exists and unseen, doing nothing to avoid disk usage DoS' % report) - drop_privileges() - write_user_coredump(pid, cwd, core_ulimit) + write_user_coredump(pid, process_start, core_ulimit) sys.exit(0) - # we prefer having a file mode of 0 while writing; this doesn't work - # for suid binaries as we completely drop privs and thus can't chmod - # them later on - if pidstat.st_uid != os.getuid(): - mode = 0o640 - else: - mode = 0 - fd = os.open(report, os.O_RDWR | os.O_CREAT | os.O_EXCL, mode) + # we prefer having a file mode of 0 while writing; + fd = os.open(report, os.O_RDWR | os.O_CREAT | os.O_EXCL, 0) reportfile = os.fdopen(fd, 'w+b') assert reportfile.fileno() > sys.stderr.fileno() @@ -763,7 +761,7 @@ error_log('Could not create report file: %s' % str(e)) sys.exit(1) - # Totally drop privs before writing out the reportfile. + # Drop privileges before writing out the reportfile. drop_privileges() info.add_user_info() @@ -774,38 +772,27 @@ try: info.write(reportfile) - if reportfile != sys.stderr: - # Ensure that the file gets written to disk in the event of an - # Upstart crash. - if info.get('ExecutablePath', '') == '/sbin/init': - reportfile.flush() - os.fsync(reportfile.fileno()) - parent_directory = os.path.dirname(report) - try: - fd = os.open(parent_directory, os.O_RDONLY) - os.fsync(fd) - finally: - os.close(fd) except IOError: - if reportfile != sys.stderr: - os.unlink(report) + os.unlink(report) raise if 'CoreDump' not in info: error_log('core dump exceeded %i MiB, dropped from %s to avoid memory overflow' % (core_size_limit / 1048576, report)) - if report and mode == 0: - # for non-suid programs, make the report writable now, when it's - # completely written - os.chmod(report, 0o640) - if reportfile != sys.stderr: - error_log('wrote report %s' % report) + + # Get privileges back so the core file can be written to root-owned + # corefile directory + recover_privileges() + + # make the report writable now, when it's completely written + os.fchmod(fd, 0o640) + error_log('wrote report %s' % report) # Check if the user wants a core file. We need to create that from the # written report, as we can only read stdin once and write_user_coredump() # might abort reading from stdin and remove the written core file when # core_ulimit is > 0 and smaller than the core size. reportfile.seek(0) - write_user_coredump(pid, cwd, core_ulimit, from_report=reportfile) + write_user_coredump(pid, process_start, core_ulimit, from_report=reportfile) except (SystemExit, KeyboardInterrupt): raise diff -Nru apport-2.20.11/data/systemd/apport.conf apport-2.20.11/data/systemd/apport.conf --- apport-2.20.11/data/systemd/apport.conf 1970-01-01 00:00:00.000000000 +0000 +++ apport-2.20.11/data/systemd/apport.conf 2021-10-06 14:34:59.000000000 +0000 @@ -0,0 +1,2 @@ +d /var/lib/apport 0755 root root - +d /var/lib/apport/coredump 0755 root root 3d diff -Nru apport-2.20.11/debian/apport.install apport-2.20.11/debian/apport.install --- apport-2.20.11/debian/apport.install 2021-03-24 14:58:15.000000000 +0000 +++ apport-2.20.11/debian/apport.install 2021-10-06 14:51:52.000000000 +0000 @@ -33,6 +33,7 @@ usr/bin/apport-collect usr/bin/oem-getlogs usr/lib/pm-utils +usr/lib/tmpfiles.d/ ../../java/apport.jar usr/share/apport/ ../../java/crash.jar usr/share/apport/testsuite/ ../../java/crash.class usr/share/apport/testsuite/ diff -Nru apport-2.20.11/debian/changelog apport-2.20.11/debian/changelog --- apport-2.20.11/debian/changelog 2021-08-26 14:55:40.000000000 +0000 +++ apport-2.20.11/debian/changelog 2021-10-18 11:48:31.000000000 +0000 @@ -1,3 +1,16 @@ +apport (2.20.11-0ubuntu65.4) hirsute-security; urgency=medium + + * SECURITY UPDATE: Privilege escalation via core files + - refactor privilege dropping and create core files in a well-known + directory in apport/fileutils.py, apport/report.py, data/apport, + test/test_fileutils.py, test/test_report.py, + test/test_signal_crashes.py, test/test_ui.py. + - use systemd-tmpfiles to create and manage the well-known core file + directory in setup.py, data/systemd/apport.conf, + debian/apport.install. + + -- Marc Deslauriers Mon, 18 Oct 2021 07:48:31 -0400 + apport (2.20.11-0ubuntu65.3) hirsute-security; urgency=medium * SECURITY UPDATE: Arbitrary file read (LP: #1934308) diff -Nru apport-2.20.11/setup.py apport-2.20.11/setup.py --- apport-2.20.11/setup.py 2021-03-24 14:58:15.000000000 +0000 +++ apport-2.20.11/setup.py 2021-10-06 14:37:34.000000000 +0000 @@ -117,9 +117,13 @@ systemd_unit_dir = subprocess.check_output( ['pkg-config', '--variable=systemdsystemunitdir', 'systemd'], universal_newlines=True).strip() + systemd_tmpfiles_dir = subprocess.check_output( + ['pkg-config', '--variable=tmpfilesdir', 'systemd'], + universal_newlines=True).strip() except subprocess.CalledProcessError: # hardcoded fallback path systemd_unit_dir = '/lib/systemd/system' + systemd_tmpfiles_dir = '/usr/lib/tmpfiles.d' DistUtilsExtra.auto.setup( name='apport', @@ -135,7 +139,8 @@ ('share/apport', ['gtk/apport-gtk', 'kde/apport-kde']), ('lib/pm-utils/sleep.d/', glob('pm-utils/sleep.d/*')), ('/lib/udev/rules.d', glob('udev/*.rules')), - (systemd_unit_dir, glob('data/systemd/*')), + (systemd_unit_dir, glob('data/systemd/*.s*')), + (systemd_tmpfiles_dir, glob('data/systemd/*.conf')), ] + optional_data_files, cmdclass=cmdclass ) diff -Nru apport-2.20.11/test/test_fileutils.py apport-2.20.11/test/test_fileutils.py --- apport-2.20.11/test/test_fileutils.py 2021-03-24 14:58:15.000000000 +0000 +++ apport-2.20.11/test/test_fileutils.py 2021-10-18 11:48:31.000000000 +0000 @@ -488,6 +488,109 @@ self.assertEqual(uid, 1000) self.assertEqual(gid, 2000) + def test_get_core_path(self): + '''get_core_path()''' + + boot_id = apport.fileutils.get_boot_id() + + # Basic test + (core_name, core_path) = apport.fileutils.get_core_path( + pid=123, + exe="/usr/bin/test", + uid=234, + timestamp=222222) + expected = "core._usr_bin_test.234." + boot_id + ".123.222222" + expected_path = os.path.join(apport.fileutils.core_dir, expected) + self.assertEqual(core_name, expected) + self.assertEqual(core_path, expected_path) + + # Test dots in exe names + (core_name, core_path) = apport.fileutils.get_core_path( + pid=123, + exe="/usr/bin/test.sh", + uid=234, + timestamp=222222) + expected = "core._usr_bin_test_sh.234." + boot_id + ".123.222222" + expected_path = os.path.join(apport.fileutils.core_dir, expected) + self.assertEqual(core_name, expected) + self.assertEqual(core_path, expected_path) + + # Test no exe name + (core_name, core_path) = apport.fileutils.get_core_path( + pid=123, + exe=None, + uid=234, + timestamp=222222) + expected = "core.unknown.234." + boot_id + ".123.222222" + expected_path = os.path.join(apport.fileutils.core_dir, expected) + self.assertEqual(core_name, expected) + self.assertEqual(core_path, expected_path) + + # Test no uid + (core_name, core_path) = apport.fileutils.get_core_path( + pid=123, + exe="/usr/bin/test", + uid=None, + timestamp=222222) + expected = ("core._usr_bin_test." + str(os.getuid()) + "." + + boot_id + ".123.222222") + expected_path = os.path.join(apport.fileutils.core_dir, expected) + self.assertEqual(core_name, expected) + self.assertEqual(core_path, expected_path) + + def test_clean_core_directory(self): + '''clean_core_directory()''' + + fake_uid = 5150 + extra_core_files = 4 + num_core_files = apport.fileutils.max_corefiles_per_uid + extra_core_files + + # Create some test files + for x in range(num_core_files): + (core_name, core_path) = apport.fileutils.get_core_path( + pid=123 + x, + exe="/usr/bin/test", + uid=fake_uid, + timestamp=222222 + x) + with open(core_path, 'w') as fd: + fd.write('Some stuff') + time.sleep(1) + + # Create a file with a different uid + (core_name, core_path) = apport.fileutils.get_core_path( + pid=231, + exe="/usr/bin/test", + uid=fake_uid + 1, + timestamp=333333) + with open(core_path, 'w') as fd: + fd.write('Some stuff') + + # Make sure we have the proper number of test files + self.assertEqual(num_core_files, + len(apport.fileutils.find_core_files_by_uid(fake_uid))) + self.assertEqual(1, + len(apport.fileutils.find_core_files_by_uid(fake_uid + 1))) + + # Clean out the directory + apport.fileutils.clean_core_directory(fake_uid) + + # Make sure we have the proper number of test files. We should + # have one less than max_corefiles_per_uid. + self.assertEqual(apport.fileutils.max_corefiles_per_uid - 1, + len(apport.fileutils.find_core_files_by_uid(fake_uid))) + self.assertEqual(1, + len(apport.fileutils.find_core_files_by_uid(fake_uid + 1))) + + # Make sure we deleted the oldest ones + for x in range(apport.fileutils.max_corefiles_per_uid - 1): + offset = extra_core_files + x + 1 + (core_name, core_path) = apport.fileutils.get_core_path( + pid=123 + offset, + exe="/usr/bin/test", + uid=fake_uid, + timestamp=222222 + offset) + self.assertTrue(os.path.exists(core_path)) + if __name__ == '__main__': unittest.main() diff -Nru apport-2.20.11/test/test_report.py apport-2.20.11/test/test_report.py --- apport-2.20.11/test/test_report.py 2021-03-24 14:58:15.000000000 +0000 +++ apport-2.20.11/test/test_report.py 2021-10-07 17:29:39.000000000 +0000 @@ -1,5 +1,6 @@ # coding: UTF-8 -import unittest, shutil, time, tempfile, os, subprocess, grp, atexit, sys, mock +import unittest, shutil, time, tempfile, os, subprocess, grp, atexit, sys +import mock, signal, resource try: from cStringIO import StringIO @@ -728,32 +729,51 @@ def test_add_gdb_info_script(self): '''add_gdb_info() with a script.''' + # This needs to handle different bash locations across releases + # to get the core filename right + shell = os.path.realpath('/bin/bash') + (fd, script) = tempfile.mkstemp() - coredump = os.path.join(os.path.dirname(script), 'core') - assert not os.path.exists(coredump) try: os.close(fd) # create a test script which produces a core dump for us with open(script, 'w') as fd: - fd.write('''#!/bin/bash + fd.write('''#!%s cd `dirname $0` ulimit -c unlimited kill -SEGV $$ -''') +''' % shell) os.chmod(script, 0o755) # call script and verify that it gives us a proper ELF core dump - assert subprocess.call([script]) != 0 - self._validate_core(coredump) + pid = os.fork() + if pid == 0: + os.dup2(os.open('/dev/null', os.O_WRONLY), sys.stdout.fileno()) + sys.stdin.close() + os.setsid() + resource.setrlimit(resource.RLIMIT_CORE, (-1, -1)) + os.chdir(apport.fileutils.report_dir) + os.execv(script, [script]) + assert False, 'Could not execute ' + script + + time.sleep(0.5) + + (core_name, core_path) = apport.fileutils.get_core_path(pid, + shell) + os.kill(pid, signal.SIGSEGV) + os.waitpid(pid, 0) + # Otherwise the core dump is empty. + time.sleep(0.5) + + self._validate_core(core_path) pr = apport.report.Report() pr['InterpreterPath'] = '/bin/bash' pr['ExecutablePath'] = script - pr['CoreDump'] = (coredump,) + pr['CoreDump'] = (core_path,) pr.add_gdb_info() finally: - os.unlink(coredump) os.unlink(script) self._validate_gdb_fields(pr) @@ -765,9 +785,11 @@ If these come from an assert(), the report should have the assertion message. Otherwise it should be marked as not reportable. ''' + # abort with assert (fd, script) = tempfile.mkstemp() - assert not os.path.exists('core') + script_bin = script + '.bin' + try: os.close(fd) @@ -778,23 +800,41 @@ #include int main() { assert(1 < 0); } EOF -ulimit -c unlimited -$0.bin 2>/dev/null ''') os.chmod(script, 0o755) - # call script and verify that it gives us a proper ELF core dump - assert subprocess.call([script]) != 0 - self._validate_core('core') + # build the crashing binary + subprocess.call([script]) + + # call binary and verify that it gives us a proper ELF core dump + pid = os.fork() + if pid == 0: + os.dup2(os.open('/dev/null', os.O_WRONLY), sys.stdout.fileno()) + sys.stdin.close() + os.setsid() + resource.setrlimit(resource.RLIMIT_CORE, (-1, -1)) + os.chdir(apport.fileutils.report_dir) + os.execv(script_bin, [script_bin]) + assert False, 'Could not execute ' + script_bin + + time.sleep(0.5) + + (core_name, core_path) = apport.fileutils.get_core_path(pid, + script_bin) + os.kill(pid, signal.SIGSEGV) + os.waitpid(pid, 0) + # Otherwise the core dump is empty. + time.sleep(0.5) + + self._validate_core(core_path) pr = apport.report.Report() - pr['ExecutablePath'] = script + '.bin' - pr['CoreDump'] = ('core',) + pr['ExecutablePath'] = script_bin + pr['CoreDump'] = (core_path,) pr.add_gdb_info() finally: os.unlink(script) - os.unlink(script + '.bin') - os.unlink('core') + os.unlink(script_bin) self._validate_gdb_fields(pr) self.assertIn(":2: main: Assertion `1 < 0' failed.", pr['AssertionMessage']) @@ -804,7 +844,8 @@ # abort with internal error (fd, script) = tempfile.mkstemp() - assert not os.path.exists('core') + script_bin = script + '.bin' + try: os.close(fd) @@ -819,23 +860,41 @@ return 0; } EOF -ulimit -c unlimited -LIBC_FATAL_STDERR_=1 $0.bin aaaaaaaaaaaaaaaa 2>/dev/null ''') os.chmod(script, 0o755) - # call script and verify that it gives us a proper ELF core dump - assert subprocess.call([script]) != 0 - self._validate_core('core') + # build the crashing binary + subprocess.call([script]) + + # call binary and verify that it gives us a proper ELF core dump + pid = os.fork() + if pid == 0: + os.dup2(os.open('/dev/null', os.O_WRONLY), sys.stdout.fileno()) + sys.stdin.close() + os.setsid() + resource.setrlimit(resource.RLIMIT_CORE, (-1, -1)) + os.chdir(apport.fileutils.report_dir) + os.execv(script_bin, [script_bin, 'aaaaaaaaaaaaaaaa']) + assert False, 'Could not execute ' + script_bin + + time.sleep(0.5) + + (core_name, core_path) = apport.fileutils.get_core_path(pid, + script_bin) + os.kill(pid, signal.SIGSEGV) + os.waitpid(pid, 0) + # Otherwise the core dump is empty. + time.sleep(0.5) + + self._validate_core(core_path) pr = apport.report.Report() - pr['ExecutablePath'] = script + '.bin' - pr['CoreDump'] = ('core',) + pr['ExecutablePath'] = script_bin + pr['CoreDump'] = (core_path,) pr.add_gdb_info() finally: os.unlink(script) - os.unlink(script + '.bin') - os.unlink('core') + os.unlink(script_bin) self._validate_gdb_fields(pr) self.assertIn("** buffer overflow detected ***: terminated", @@ -846,7 +905,8 @@ # abort without assertion (fd, script) = tempfile.mkstemp() - assert not os.path.exists('core') + script_bin = script + '.bin' + try: os.close(fd) @@ -857,23 +917,41 @@ #include int main() { abort(); } EOF -ulimit -c unlimited -$0.bin 2>/dev/null ''') os.chmod(script, 0o755) - # call script and verify that it gives us a proper ELF core dump - assert subprocess.call([script]) != 0 - self._validate_core('core') + # build the crashing binary + subprocess.call([script]) + + # call binary and verify that it gives us a proper ELF core dump + pid = os.fork() + if pid == 0: + os.dup2(os.open('/dev/null', os.O_WRONLY), sys.stdout.fileno()) + sys.stdin.close() + os.setsid() + resource.setrlimit(resource.RLIMIT_CORE, (-1, -1)) + os.chdir(apport.fileutils.report_dir) + os.execv(script_bin, [script_bin]) + assert False, 'Could not execute ' + script_bin + + time.sleep(0.5) + + (core_name, core_path) = apport.fileutils.get_core_path(pid, + script_bin) + os.kill(pid, signal.SIGSEGV) + os.waitpid(pid, 0) + # Otherwise the core dump is empty. + time.sleep(0.5) + + self._validate_core(core_path) pr = apport.report.Report() - pr['ExecutablePath'] = script + '.bin' - pr['CoreDump'] = ('core',) + pr['ExecutablePath'] = script_bin + pr['CoreDump'] = (core_path,) pr.add_gdb_info() finally: os.unlink(script) - os.unlink(script + '.bin') - os.unlink('core') + os.unlink(script_bin) self._validate_gdb_fields(pr) self.assertFalse('AssertionMessage' in pr, pr.get('AssertionMessage')) diff -Nru apport-2.20.11/test/test_signal_crashes.py apport-2.20.11/test/test_signal_crashes.py --- apport-2.20.11/test/test_signal_crashes.py 2021-03-24 14:58:15.000000000 +0000 +++ apport-2.20.11/test/test_signal_crashes.py 2021-10-07 14:28:19.000000000 +0000 @@ -382,6 +382,11 @@ # crash our test process and let it write a core file resource.setrlimit(resource.RLIMIT_CORE, (-1, -1)) pid = self.create_test_process() + + # get the name of the core file + (core_name, core_path) = apport.fileutils.get_core_path(pid, + test_executable) + os.kill(pid, signal.SIGSEGV) # replace report with the crafted one above as soon as it exists and @@ -400,13 +405,11 @@ os.sync() # verify that we get the original core, not the injected one - with open('core', 'rb') as f: + with open(core_path, 'rb') as f: core = f.read() self.assertNotIn(b'pwned', core) self.assertGreater(len(core), 10000) - os.unlink('core') - def test_limit_size(self): '''core dumps are capped on available memory size''' @@ -894,6 +897,11 @@ ''' self.assertFalse(os.path.exists('core'), '%s/core already exists, please clean up first' % os.getcwd()) pid = self.create_test_process(check_running, command, uid=uid, args=args) + + (core_name, core_path) = apport.fileutils.get_core_path(pid, + command, + uid) + if sleep > 0: time.sleep(sleep) if killer_id: @@ -947,16 +955,15 @@ self.assertEqual(subprocess.call(['pidof', command]), 1, 'no running test executable processes') - core_path = '%s/' % os.getcwd() if core_location: - core_path = '%s/' % core_location - core_path += 'core' + core_path = '%s/core' % core_location + if expect_corefile: self.assertTrue(os.path.exists(core_path), 'leaves wanted core file') try: # check core file permissions st = os.stat(core_path) - self.assertEqual(stat.S_IMODE(st.st_mode), 0o600, 'core file has correct permissions') + self.assertEqual(stat.S_IMODE(st.st_mode), 0o400, 'core file has correct permissions') if expect_corefile_owner is not None: self.assertEqual(st.st_uid, expect_corefile_owner, 'core file has correct owner') diff -Nru apport-2.20.11/test/test_ui.py apport-2.20.11/test/test_ui.py --- apport-2.20.11/test/test_ui.py 2021-03-24 14:58:15.000000000 +0000 +++ apport-2.20.11/test/test_ui.py 2021-10-07 12:36:09.000000000 +0000 @@ -743,7 +743,7 @@ self.assertEqual(self.ui.msg_severity, None) self.assertTrue(self.ui.present_details_shown) - def _gen_test_crash(self): + def _gen_test_crash(self, uid=None): '''Generate a Report with real crash data''' # create a test executable @@ -770,13 +770,19 @@ r.add_os_info() # generate a core dump - coredump = os.path.join(apport.fileutils.report_dir, 'core') + + if uid is None: + uid = os.getuid() + + (core_name, core_path) = apport.fileutils.get_core_path(pid, + test_executable, + uid) os.kill(pid, signal.SIGSEGV) os.waitpid(pid, 0) # Otherwise the core dump is empty. time.sleep(0.5) - assert os.path.exists(coredump) - r['CoreDump'] = (coredump,) + assert os.path.exists(core_path) + r['CoreDump'] = (core_path,) return r @@ -1486,7 +1492,7 @@ os.getuid = lambda: 1234 try: - r = self._gen_test_crash() + r = self._gen_test_crash(orig_getuid()) r['ProcInfo1'] = 'That was Joe (Hacker and friends' r['ProcInfo2'] = 'Call +1 234!' r['ProcInfo3'] = '(Hacker should stay'