Apport get_config incorrectly drops privileges

[Marc Deslauriers]

Privilege dropping code here drops uid before gid instead of the correct order of gid before uid. Likely this code fails and is caught by the try statement:

https://git.launchpad.net/ubuntu/+source/apport/tree/apport/fileutils.py?h=applied/ubuntu/focal-devel&id=94d98694e19b5614f3a764a9e4dcf171ad5039a5#n339

[Jamie Strandboge (jdstrand)]

Please also do the python equivalant of setgroups(0, NULL) before dropping the gid (you could also use a one element list that is the dropped-to group). To add them back, you'll need to do the setgroups() after raising the uid. Drop order is setgroups(), setegid(), seteuid(); raise order is seteuid(), setegid(), setgroups().

[Marc Deslauriers (mdeslaur)]

What groups do you think the root user would be in that would require doing that?

[Jamie Strandboge (jdstrand)]

Assuming we aren't talking about a setuid application, it's possible for the supplementary groups to contain 'root' (eg, run under sudo, ssh root logins, etc).

If we are talking about a setuid application, we shouldn't blindly drop them (and there is no reason to since root isn't going to be part of the group set in a setuid application anyway).

Please verify that the code in question isn't running under setuid (IME it is not) and assuming it is not running under setuid, please drop the supplementary groups to ensure we're covered all the cases when temporarily dropping and the group is present.

[Jamie Strandboge (jdstrand)]

Marc and I also discussed this on IRC wrt setgroups(0, NULL) and initgroups() to the user's groups. IME, if the requirements/expectations of the code in question is fine with just the primary group, setgroups(0, NULL) is safest, but if that isn't the case, we should consider initgroups.

[Marc Deslauriers (mdeslaur)]

This looks wrong too:

https://git.launchpad.net/ubuntu/+source/apport/tree/apport/report.py#n1048

[Marc Deslauriers (mdeslaur)]

Perhaps we should simply drop support for those home directory files. It looks like the main one in this bug hasn't worked in a while and perhaps by fixing it we will introduce more security issues that were masked by it being broken...

[Marc Deslauriers (mdeslaur)]

I guess we can't drop support for the second one.

[Marc Deslauriers (mdeslaur)]

OK, a little more information about the main issue in this bug. The privilege dropping code was introduced to fix CVE-2019-11481, from bug 1830862, which had two parts:

- File was read as root
- There was no limit on the size of the file so a symlink to /dev/zero would cause apport to hang

The patch fixed #1. The patch did not specifically address #2, but since the privilege dropping code was broken resulting in a failure, the result was that #2 was no longer an issue.

Fixing the privilege dropping code will re-introduce #2.

The file is parsed using ConfigParser(), and there is no API to limit the size of the file being read. There are two possible ways around the issue:

1- Implement a simple key-value parser in a few lines of code that includes a limit on the number of lines read from the file. Since apport only uses the [main] header, parsing headers is not needed.

2- Read the file with a line limit before passing it to ConfigParser().

I suspect #1 would give us more control in limiting and addressing further attacks in the future.

[Launchpad Janitor (janitor)]

This bug was fixed in the package apport - 2.20.11-0ubuntu50.1

---------------
apport (2.20.11-0ubuntu50.1) groovy-security; urgency=medium

  * Various security hardening fixes (LP: #1903332)
    - apport/fileutils.py: drop privileges in the correct order, limit
      settings file size.
    - apport/apport/report.py: properly drop privileges, limit ignore file
      size.
    - data/apport: drop supplemental groups.

 -- Marc Deslauriers <email address hidden> Tue, 10 Nov 2020 15:03:57 -0500

[Launchpad Janitor (janitor)]

This bug was fixed in the package apport - 2.20.11-0ubuntu27.12

---------------
apport (2.20.11-0ubuntu27.12) focal-security; urgency=medium

  * Various security hardening fixes (LP: #1903332)
    - apport/fileutils.py: drop privileges in the correct order, limit
      settings file size.
    - apport/apport/report.py: properly drop privileges, limit ignore file
      size.
    - data/apport: drop supplemental groups.

 -- Marc Deslauriers <email address hidden> Tue, 10 Nov 2020 15:03:57 -0500

[Launchpad Janitor (janitor)]

This bug was fixed in the package apport - 2.20.9-0ubuntu7.20

---------------
apport (2.20.9-0ubuntu7.20) bionic-security; urgency=medium

  * Various security hardening fixes (LP: #1903332)
    - apport/fileutils.py: drop privileges in the correct order, limit
      settings file size.
    - apport/apport/report.py: properly drop privileges, limit ignore file
      size.
    - data/apport: drop supplemental groups.

 -- Marc Deslauriers <email address hidden> Tue, 10 Nov 2020 15:03:57 -0500