Skip to content

Commit

Permalink
security update: sync with 2.20.11-0ubuntu44
Browse files Browse the repository at this point in the history
* SECURITY UPDATE: information disclosure issue (LP: #1885633)
  - data/apport: also drop gid when checking if user session is closing.
  - CVE-2020-11936
* SECURITY UPDATE: crash via malformed ignore file (LP: #1877023)
  - apport/report.py: don't crash on malformed mtime values.
  - CVE-2020-15701
* SECURITY UPDATE: TOCTOU in core file location - data/apport: make sure
  the process hasn't been replaced after Apport has started.
  - CVE-2020-15702
* apport/ui.py, test/test_ui.py: make sure a PID is specified when using
  --hanging (LP: #1876659)
  • Loading branch information
Marc Deslauriers committed Jul 31, 2020
1 parent 73c9c16 commit 19d0cad
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 11 deletions.
10 changes: 10 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,16 @@ detailed list of changes, please see ChangeLog.
not cause Apport to change the ownership of other files via a
symlink attack.
- CVE-2020-8833
* SECURITY UPDATE: information disclosure issue (LP: #1885633)
- data/apport: also drop gid when checking if user session is closing.
- CVE-2020-11936
* SECURITY UPDATE: crash via malformed ignore file (LP: #1877023)
- apport/report.py: don't crash on malformed mtime values.
- CVE-2020-15701
* SECURITY UPDATE: TOCTOU in core file location
- data/apport: make sure the process hasn't been replaced after Apport
has started.
- CVE-2020-15702
* backends/packaging-apt-dpkg.py: Utilize a release and architecture specific
contents mapping.
* test/test_backend_apt_dpkg.py: Update the test as we are using a contents
Expand Down
11 changes: 7 additions & 4 deletions apport/report.py
Original file line number Diff line number Diff line change
Expand Up @@ -1129,10 +1129,13 @@ def check_ignored(self):
return False

# search for existing entry and update it
for ignore in dom.getElementsByTagName('ignore'):
if ignore.getAttribute('program') == self['ExecutablePath']:
if float(ignore.getAttribute('mtime')) >= cur_mtime:
return True
try:
for ignore in dom.getElementsByTagName('ignore'):
if ignore.getAttribute('program') == self['ExecutablePath']:
if float(ignore.getAttribute('mtime')) >= cur_mtime:
return True
except (ValueError, KeyError):
pass

return False

Expand Down
9 changes: 8 additions & 1 deletion apport/ui.py
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,12 @@ def run_hang(self, pid):
mark the report for uploading.
'''
self.report = apport.Report('Hang')

if not self.options.pid:
self.ui_error_message(_('No PID specified'),
_('You need to specify a PID. See --help for more information.'))
return False

try:
self.report.add_proc_info(pid)
except ValueError as e:
Expand Down Expand Up @@ -701,7 +707,7 @@ def run_argv(self):
if self.options.symptom:
self.run_symptom()
return True
elif hasattr(self.options, 'pid') and self.options.hanging:
elif self.options.hanging:
self.run_hang(self.options.pid)
return True
elif self.options.filebug:
Expand Down Expand Up @@ -763,6 +769,7 @@ def parse_argv_update(self):
self.options.filebug = False
self.options.crash_file = None
self.options.version = None
self.options.hanging = False
self.args = []

def parse_argv(self):
Expand Down
45 changes: 41 additions & 4 deletions data/apport
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,30 @@ def get_pid_info(pid):
cwd = os.open('cwd', os.O_RDONLY | os.O_PATH | os.O_DIRECTORY, dir_fd=proc_pid_fd)


def get_starttime(contents):
'''Extracts the starttime from the contents of a stat file'''

# 22nd field in a stat file is the time the process started after
# system boot in clock ticks.
return int(contents.split()[21])


def get_process_starttime():
'''Get the starttime of the process using proc_pid_fd'''

with open("stat", opener=proc_pid_opener) as stat_file:
contents = stat_file.read()
return get_starttime(contents)


def get_apport_starttime():
'''Get the Apport process starttime'''

with open("/proc/%s/stat" % os.getpid()) as stat_file:
contents = stat_file.read()
return get_starttime(contents)


def drop_privileges(real_only=False):
'''Change user and group to real_[ug]id
Expand Down Expand Up @@ -266,9 +290,11 @@ def is_closing_session(uid):
error_log('is_closing_session(): no DBUS_SESSION_BUS_ADDRESS in environment')
return False

orig_uid = os.geteuid()
os.setresuid(-1, os.getuid(), -1)
euid = os.geteuid()
egid = os.getegid()
try:
os.setegid(os.getgid())
os.seteuid(os.getuid())
gdbus = subprocess.Popen(['/usr/bin/gdbus', 'call', '-e', '-d',
'org.gnome.SessionManager', '-o', '/org/gnome/SessionManager', '-m',
'org.gnome.SessionManager.IsSessionRunning'], stdout=subprocess.PIPE,
Expand All @@ -280,7 +306,8 @@ def is_closing_session(uid):
error_log('gdbus call failed, cannot determine running session: ' + str(e))
return False
finally:
os.setresuid(-1, orig_uid, -1)
os.seteuid(euid)
os.setegid(egid)
error_log('debug: session gdbus call: ' + out.decode('UTF-8'))
if out.startswith(b'(false,'):
return True
Expand Down Expand Up @@ -551,6 +578,16 @@ try:

get_pid_info(pid)

# Check if the process was replaced after the crash happened.
# Ideally we'd use the time of dump value provided by the kernel,
# but this would be a backwards-incompatible change that would
# require adding support to apport outside and inside of containers.
apport_start = get_apport_starttime()
process_start = get_process_starttime()
if process_start > apport_start:
error_log('process was replaced after Apport started, ignoring')
sys.exit(0)

# Partially drop privs to gain proper os.access() checks
drop_privileges(True)

Expand Down Expand Up @@ -607,8 +644,8 @@ 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.seteuid(os.getuid())
os.setegid(os.getgid())
os.seteuid(os.getuid())
info.add_proc_info(proc_pid_fd=proc_pid_fd)
finally:
os.seteuid(euid)
Expand Down
4 changes: 2 additions & 2 deletions test/test_ui.py
Original file line number Diff line number Diff line change
Expand Up @@ -2099,10 +2099,10 @@ def run(report, ui):
# update existing report
_chk('apport-collect', '1234',
{'filebug': False, 'package': None, 'crash_file': None, 'symptom':
None, 'update_report': 1234, 'tag': []})
None, 'update_report': 1234, 'tag': [], 'hanging': False})
_chk('apport-update-bug', '1234',
{'filebug': False, 'package': None, 'crash_file': None, 'symptom':
None, 'update_report': 1234, 'tag': []})
None, 'update_report': 1234, 'tag': [], 'hanging': False})

def test_parse_argv_apport_bug(self):
'''parse_args() option inference when invoked as *-bug'''
Expand Down

0 comments on commit 19d0cad

Please sign in to comment.