Desktop setuid cores readable by non-privileged user

[Martin Carpenter]

Elsewhere I have been working on a sensitive information leak via core dump generated by gcore(1).

The sensitive information in question is read by a stock setuid root binary executed by a non-privileged user. On Ubuntu Desktop fs.suid_dumpable=2. Referencing https://www.kernel.org/doc/Documentation/sysctl/fs.txt:

2 - (suidsafe) - any binary which normally would not be dumped is dumped
 anyway, but only if the "core_pattern" kernel sysctl is set to
 either a pipe handler or a fully qualified path. (For more details
 on this limitation, see CVE-2006-2451.) This mode is appropriate
 when administrators are attempting to debug problems in a normal
 environment, and either have a core dump pipe handler that knows
 to treat privileged core dumps with care, or specific directory
 defined for catching core dumps. If a core dump happens without
 a pipe handler or fully qualifid path, a message will be emitted
 to syslog warning about the lack of a correct setting.

NB "treat privileged core dumps with care".

On a stock Desktop 12.04 LTS install:

    kernel.core_pattern = |/usr/share/apport/apport %p %s %c

apport dutifully dumps the core and this is readable (0660, user:user) by the invoking user, whereas it should be something like 0440, root:root. I believe this to be a bug in apport.

TRUNK FIX: http://bazaar.launchpad.net/~apport-hackers/apport/trunk/revision/2723
Backports for older releases available as attachments here.

[Martin Carpenter (a-mcarpenter)]

Please find a minimal test case below (replaces ping(1) with a call to abort(3)). I've tried this on 3 desktops now and got different results each time (core perms combinations of martin:martin/root:martin/640/660) but in all cases core was readable by the non-privileged user.

I also wanted to explicitly state that this is not a problem on Ubuntu Server 12.04, where fs.suid_dumpable=0.

martin@desktop:~/apport$ cat > foo.c

#include <stdlib.h>

int main(int argc, char *argv[])
{
    abort();
}
^D
martin@desktop:~/apport$ make foo
cc foo.c -o foo
martin@desktop:~/apport$ aptitude search iputils-ping
i iputils-ping - Tools to test the reachability of network hosts
p iputils-ping:i386 - Tools to test the reachability of network hosts
martin@desktop:~/apport$ sudo mv /bin/ping /bin/ping.org
martin@desktop:~/apport$ sudo cp ./foo /bin/ping
martin@desktop:~/apport$ sudo chown root:root /bin/ping
martin@desktop:~/apport$ sudo chmod 4755 /bin/ping
martin@desktop:~/apport$ ls -l /bin/ping.org /bin/ping
-rwsr-xr-x. 1 root root 8376 Oct 21 10:13 /bin/ping
-rwsr-xr-x. 1 root root 35712 Nov 8 2011 /bin/ping.org
martin@desktop:~/apport$ ulimit -c unlimited
martin@desktop:~/apport$ ./foo
Aborted (core dumped)
martin@desktop:~/apport$ ls -l core
-rw-r-----. 1 martin martin 233472 Oct 21 10:14 core
martin@desktop:~/apport$

[Martin Pitt (pitti)]

Thanks for spotting this! This is a much simpler test case:

$ (ping www.ubuntu.com &); sleep 1; sudo pkill -SEGV ping; sleep 1; ls -l /var/crash/_bin_ping.*.crash
-rw-r----- 1 martin whoopsie 57649 Okt 23 12:53 /var/crash/_bin_ping.1000.crash

Indeed most of /proc/<pid>/* are owned by root:root in this case, and "status" shows euid=0. I agree that we should make the .crash file owned by the effective instead of real UID.

[Martin Pitt (pitti)]

"Low" for lucid as we disabled Apport by default on releases before precise.

Security team, should this get a CVE? If so, can you please assign one? Apport is not being used that widely outside of Ubuntu, but some distros at least used it in the past.

[Marc Deslauriers (mdeslaur)]

This is CVE-2013-1067

[Martin Carpenter (a-mcarpenter)]

Can we also set default fs.suid_dumpable=0? That's fundamental here and I don't see any good reason for it to be 2 by default.

[Marc Deslauriers (mdeslaur)]

Setting it to 0 would prevent us from getting automated bug reports for a lot of important processes. I don't think that would be a good option for our users.

[Martin Pitt (pitti)]

> Indeed most of /proc/<pid>/* are owned by root:root in this case, and "status" shows euid=0. I agree that we should make the .crash file owned by the effective instead of real UID.

Correction: ping drops its euid back to the calling user right after program start, so this does not actually affect a setuid program which keeps root privileges. For those, reports are owned by "root" already as they should.

So this only affects suid programs which switch back to the user; for ping, /proc/pid/status shows that real and effective uid are "1000" (or whichever user called it), just the saved uid is 0, i. e. ping uses setresuid(n, n, -1) instead of dropping the possibility of ever going back (setuid(n)).

So the impact of this is actually quite low.

[Martin Carpenter (a-mcarpenter)]

Martin, I came here via ssh-keysign(8): the private host key is exposed in the resulting core file (as written out by apport). There are bound to be other suid applications that drop privileges in this way. This is, after all, the sane thing for an application to do. I don't think you can therefore claim that his has low impact.

Marc, I was already surprised to see the b64'd core file in the crash report. Although this is noted in the pop-up and apport wiki I don't think the claimed mitigations ("we ask the user and treat the data carefully") are adequate (in particular for a desktop OS aimed at the non-technical user). That those crashes might contain other users' data via setuid does not improve the situation(!) so I don't think suid_dumpable=2 is a justifiable default.

[Martin Pitt (pitti)]

Nevermind, I wrote a wrong test: I created a test for the .crash file, but not for the "core" file. I'm doing that now, we should actually verify both.

[Martin Pitt (pitti)]

It seems the kernel already does a fine job on setting an appropriate owner for /proc/<pid>/stat, so instead of querying the /proc/pid directory (which is owned by the user for the "ping" case), this changes the query to /proc/<pid>/stat. This patch also adds two new tests, one for "setuid and keeps privileges" (which is already working, and the much more important case), and for "setuid and drops privileges" (which is this bug, and slightly less important).

This patch is against trunk, and applies as-is (except the NEWS hunk) to raring, saucy, and trusty.

[Martin Pitt (pitti)]

This is a quantal backport.

[Martin Pitt (pitti)]

This is a precise backport.

[Martin Pitt (pitti)]

This is a lucid backport, for the patch only. Lucid's test suite is very different and the new tests don't backport at all there. I'd rather test this manually on lucid, if you want to issue an USN for lucid at all: we only support the server part still, core dumps are already 0600 on lucid (they were changed to 0640 in a later release), and apport is disabled on lucid by default.

Security team, how to proceed here? I did not commit anything to public repositories. Is this worth keeping secret and doing a coordinated release, or should I just commit everything to trunk and the packaging branches, send you the updated .dsc or debdiffs (depending on what you prefer), and let you handle the upload?

[Marc Deslauriers (mdeslaur)]

Thanks for the patches Martin, I'll take it from here.

I'll prepare, test, and release the updates, please leave the bug private until I release them.

Thanks!

[Martin Pitt (pitti)]

Thanks Marc! Could you please leave the trusty one to me? I'm going to make a new upstream release at the same time then and upload that to trusty (I have some other fixes queued there).

[Marc Deslauriers (mdeslaur)]

Sure!

[Launchpad Janitor (janitor)]

This bug was fixed in the package apport - 2.0.1-0ubuntu17.6

---------------
apport (2.0.1-0ubuntu17.6) precise-security; urgency=low

  * SECURITY UPDATE: incorrect permissions on setuid process core dumps
    (LP: #1242435)
    - use correct permissions when writing the core file in data/apport,
      added test to test/test_signal_crashes.py.
    - Thanks to Martin Pitt for the patch!
    - CVE-2013-1067
 -- Marc Deslauriers <email address hidden> Wed, 23 Oct 2013 13:04:37 -0400

[Launchpad Janitor (janitor)]

This bug was fixed in the package apport - 2.9.2-0ubuntu8.5

---------------
apport (2.9.2-0ubuntu8.5) raring-security; urgency=low

  * SECURITY UPDATE: incorrect permissions on setuid process core dumps
    (LP: #1242435)
    - use correct permissions when writing the core file in data/apport,
      added test to test/test_signal_crashes.py.
    - Thanks to Martin Pitt for the patch!
    - CVE-2013-1067
 -- Marc Deslauriers <email address hidden> Wed, 23 Oct 2013 14:18:57 -0400

[Launchpad Janitor (janitor)]

This bug was fixed in the package apport - 2.12.5-0ubuntu2.1

---------------
apport (2.12.5-0ubuntu2.1) saucy-security; urgency=low

  * SECURITY UPDATE: incorrect permissions on setuid process core dumps
    (LP: #1242435)
    - use correct permissions when writing the core file in data/apport,
      added test to test/test_signal_crashes.py.
    - Thanks to Martin Pitt for the patch!
    - CVE-2013-1067
 -- Marc Deslauriers <email address hidden> Wed, 23 Oct 2013 12:50:57 -0400

[Launchpad Janitor (janitor)]

This bug was fixed in the package apport - 2.6.1-0ubuntu13

---------------
apport (2.6.1-0ubuntu13) quantal-security; urgency=low

  * SECURITY UPDATE: incorrect permissions on setuid process core dumps
    (LP: #1242435)
    - use correct permissions when writing the core file in data/apport,
      added test to test/test_signal_crashes.py.
    - Thanks to Martin Pitt for the patch!
    - CVE-2013-1067
 -- Marc Deslauriers <email address hidden> Wed, 23 Oct 2013 13:03:40 -0400

[Martin Pitt (pitti)]

Fixed in upstream release 2.12.6: https://launchpad.net/apport/trunk/2.12.6

[Launchpad Janitor (janitor)]

This bug was fixed in the package apport - 2.12.6-0ubuntu1

---------------
apport (2.12.6-0ubuntu1) trusty; urgency=low

  * New upstream security/bug fix release:
    - SECURITY FIX: For setuid programs which drop their privileges after
      startup, make the report and core dumps owned by root, to avoid possible
      data disclosure. Also, change core dump files to permissions "0600".
      Thanks to Martin Carpenter for discovering this!
      (CVE-2013-1067, LP: #1242435)
    - sandboxutils.needed_runtime_packages(): Create cache directory for
      Contents.gz if missing. (LP: #933199)
    - apt/dpkg: Recognize options in apt sources.list. (LP: #1238620)
  * Move Vcs-Bzr to trusty branch.
 -- Martin Pitt <email address hidden> Fri, 25 Oct 2013 06:49:19 +0200