/usr/share/apport/kernel_crashdump accesses files in insecure manner

[halfdog]

On Ubuntu Vivid Linux distribution upstart or SysV init invokes the program /usr/share/apport/kernel_crashdump at boot to prepare crash dump files for sending. This action is performed with root privileges. As the crash dump directory /var/crash/ is world writable and kernel_crashdump performs file access in unsafe manner, any local user may trigger a denial of service or escalate to root privileges. If symlink and hardlink protection is enabled (which should be the default for any modern system), only denial of service is possible.

Problematic syscall in kernel_crashdump is:

open("/var/crash/linux-image-3.19.0-18-generic.0.crash", O_WRONLY|O_CREAT|O_TRUNC|O_LARGEFILE|O_CLOEXEC, 0666) = 30
...
open("/var/crash/vmcore.log", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = 31

Thus the output file is opened unconditionally and without O_EXCL or O_NOFOLLOW. Also opening of input file does not care about links.

By sym- or hardlinking from the predictable dump file name to the vmcore.log, kernel_crashdump will recursively include its own dump as logfile, thus filling the disk. This also works with symlink and hardlink protection turned on.

By symlinking to other files (with symlink protection off), arbitrary files can be overwritten to gain root privileges.

# lsb_release -rd
Description: Ubuntu 15.04
Release: 15.04

# apt-cache policy apport
apport:
  Installed: 2.17.2-0ubuntu1.3
  Candidate: 2.17.2-0ubuntu1.3
  Version table:
 *** 2.17.2-0ubuntu1.3 0
        500 http://archive.ubuntu.com/ubuntu/ vivid-updates/main i386 Packages
        100 /var/lib/dpkg/status
     2.17.2-0ubuntu1.1 0
        500 http://archive.ubuntu.com/ubuntu/ vivid-security/main i386 Packages
     2.17.2-0ubuntu1 0
        500 http://archive.ubuntu.com/ubuntu/ vivid/main i386 Packages

See http://www.halfdog.net/Security/2015/ApportKernelCrashdumpFileAccessVulnerabilities/ for more information and follow the link on the bottom if you know what you are doing (user: InvitedOnly, pass: w0f63smR).

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Anyone helping to fix, analyze, mitigate, the security issue at
http://www.halfdog.net/Security/2015/ApportKernelCrashdumpFileAccessVulnerabilities/
to improve security is allowed to view and use this resource. It
may be passed on (including password) to other security engineers
under the same conditions at your own risk. Free circulation
of that resource is allowed as soon as password protection was
removed or when stated on the page itself.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iEYEARECAAYFAlXqzOcACgkQxFmThv7tq+7GTwCgiwCkUqsB0qiwGIktUMIPqgXY
9bYAni2R8hAZVWWrtPZ+xsDgHGgWq2gL
=Y4E5
-----END PGP SIGNATURE-----

[Marc Deslauriers (mdeslaur)]

Hi Martin,

Could you please take a look at this issue? Thanks!

[Martin Pitt (pitti)]

Thanks halfdog for discovering this! The missing O_EXCL when creating the .crash file is a real issue indeed. The "fill the disk" aspect seems less of an issue as normal users can fill up /var/crash/ anyway (of course on ext one usually has some 5% space reserved for root only), but checking for the file to not be a symlink is trivial either way.

[Martin Pitt (pitti)]

Wrt. the "symlink.crash" aspect, several hooks are affected by this. Only some (apportcheckresume and apport itself) already open the file with O_EXCL. Hooks usually use fileutils.make_report_path(), so I propose to change that to fileutils.make_report_file(), and let that open the file exclusively.

Wrt. the "symlink log" aspect, I checked the other hooks with grep -r '\[.*= (' and they seem okay. E. g. gcc_ice_hook is run as normal user, and apport itself already uses an fd. package_hook attaches log files that way, but they are specified by package-shipped hooks and are thus not user controllable.

[Martin Pitt (pitti)]

This is my proposed patch against trunk for preventing the "recursively include itself" log file issue. Note that subprocess.DEVNULL (in the test case) doesn't exist yet in Python 2, thus this will need some minor changes on backporting. I'll attach debdiffs for Ubuntu stable packages that will include both fixes, but for trunk I'd like to commit the issues separately.

[Martin Pitt (pitti)]

This patch against trunk solves the main issue with the symlink attack.

Comments?

[Martin Pitt (pitti)]

Also, do you want/need a CRD for this? This should at least get a CVE; Marc, can you please assign one?

I'll backport the patches to stables once this gets a round of review and CVE/CRD.

[halfdog (halfdog)]

The first patch will solve the issue only when the new crashdump is not only with e.g. O_CREAT|O_EXCL|O_NOFOLLOW, and then it would not be needed any more.

With current patch, a hardlink to the crash dump would still allow DOS. If crashdump open will a) fail when dump file already exists (like above) or b) unlinks it before recreating, then self-inclusion will not be possible for both sym- and hardlinks.

This would make also sense in another way: the os.walk below is in that combination far more dangerous than the plain open:

It has the same vulnerabilities as before, but as crashdump is following into subdirectories, O_NOFOLLOW is ineffective for TOCTOU kind of attacks, see [1].

Apart from that, the loop misses joining in the root element into the path, so os.walk will essentially walk some objects, but the open calls afterwards will just open something completely different.

With correct crashdump opening, even the misbehaving os.walk would have no effect on DOS (except producing more sym/hardlink issues).

Still working on validating second patch.

[1] http://www.halfdog.net/Security/2010/FilesystemRecursionAndSymlinks/

[Martin Pitt (pitti)]

halfdog [2015-09-11 20:54 -0000]:
> The first patch will solve the issue only when the new crashdump is not
> only with e.g. O_CREAT|O_EXCL|O_NOFOLLOW, and then it would not be
> needed any more.

There are some parts missing here, but I think I understand what you
mean. With the second patch, the .crash file is now opened with
O_EXCL. However, that does not do anything to prevent self-inclusion:
If you create a

  /var/crash/vmcore.log -> linux_1234.crash

then even with O_EXCL the creation of /var/crash/linux_1234.crash will
succeed. It will just try to slurp in the contents of itself. This is
now prevented with the os.O_NOFOLLOW.

I don't see how a hardlink would work: You'd have to pre-create a file
/var/crash/linux_1234.crash in order to hardlink to it, and then
kernel_crashdump would fail to open it due to the O_EXCL in the second
patch.

> With current patch, a hardlink to the crash dump would still allow DOS.

With just the first one, yes. With the second one I believe no.

> This would make also sense in another way: the os.walk below is in that
> combination far more dangerous than the plain open:
>

Indeed, I missed that. This needs to be fixed in the second patch, to
create the .crash files there atomically too. Thanks!

> Apart from that, the loop misses joining in the root element into the
> path, so os.walk will essentially walk some objects, but the open calls
> afterwards will just open something completely different.

I believe that's indentional. We don't want to put the .crash files
into the per-timestamp subdirs, but right into /var/crash/.

I'll post an updated second patch shortly.
--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)

[halfdog (halfdog)]

@O_EXCL: Oh, my mistake. Of course it will work for hardlinks also.

@os.walk: To put it in /var/crash directly is ok (design decision). But if I'm not wrong, current reading implementation will e.g. walk into

/var/crash/xxxx/012345678901

but then will try to open

/var/crash/012345678901/dmesg ...

which is a file not wher os.walk has walked before. I'm not sure, if this might not be used in some unwanted fashion.

Apart from that: going into a subdirectory will defeat the O_NOFOLLOW as this is only applied to the last component (dmesg...) but not the "directory" the file is in. But also here: os symlink protection will still work over that and you need to make os.walk timerace (so you need an attacker controlled unprivileged process while kernel_crashdump is running)

[Martin Pitt (pitti)]

@halfdog: Indeed, the os.walk() looks pretty broken. It can potentially find stuff in subdirs of /var/crash/<timestamp>, and completely discards os.walk's "root" return value. It also uses "vmcore_root" which is just an alias for apport.fileutils.report_dir, which is confusing for reading the code.

@Louis: I subscribed you as he authored the kernel_crashdump tool for kdump-tools. This is still an undisclosed bug, please don't mention any of this outside of this bug report until this gets public.

I was wondering how kdump-tools works in the first place: It seems that there is no /var/crash/vmcore or /var/crash/<timestamp>/vmcore at all, even though the comment says "kdump-tools has moved vmcore to timestamped dir"? Both the test case and kernel_crashdump only handle picking up the "dmesg" file from /var/crash/<timestamp>/, but no .log file nor actual vmcore file? Is this missing from kernel_crashdump and the test, or intended and the commentary is just wrong? This isn't clear from https://blueprints.launchpad.net/ubuntu/+spec/servercloud-r-kdump-tool .

Can you please do an "ls -lR /var/crash" after a typical crash that is handled by kdump-tools, and show the result here?

[Martin Pitt (pitti)]

This is the revised patch against trunk for preventing the "recursively include itself" log file issue. This now covers the "kdump-tools" case as well. This assumes that it's intended to only ever include that dmesg.* file there, and *not* an actual "vmcore" file. Louis, please confirm.

[Martin Pitt (pitti)]

This is the revised patch for opening reports exclusively.

[Louis Bouchard (louis)]

Hi Martin, halfdog,

Just to be precise, I didn't author kernel_crashdump, just adapted it to the use of kdump-tools instead of custom scripts.

Here is the content of /var/crash after a clean install & addition of the linux-crashdump meta-package that will install the necessary bits to enable kernel crash dumps (i.e. makedumpfile, kexec-tools, kdump-tools & a few more).

# find /var/crash
/var/crash
/var/crash/kexec_cmd

This is the content of /var/crash just after the kernel dump capture and reboot, but BEFORE /usr/share/apport/kernel_crashdump gets executed ( boot was stopped by the 'debug break=init' boot parameter) :

(initramfs) find /root/var/crash
/root/var/crash
/root/var/crash/201509160921
/root/var/crash/201509160921/dmesg.201509160921
/root/var/crash/201509160921/dump.201509160921
/root/var/crash/kexec_cmd

This is the content of /var/crash AFTER kernel_crashdump execution :

$ find /var/crash
/var/crash
/var/crash/linux-image-3.13.0-62-generic 3.13.0-62.102-201509160921.crash
/var/crash/201509160921
/var/crash/201509160921/dmesg.201509160921
/var/crash/201509160921/dump.201509160921
/var/crash/kexec_cmd

The comment means that, starting with Raring, there is no longer any /var/crash/vmcore generated but rather what we see above, which is /var/crash/{timestamp}/dump.{timestamp}

Now looking at the tests & what I modified, I can see the misleading bits.

vmcore & vmcore.log tests are part of the test_kernel_crashdump_kexec which tests the Pre-Raring way of gathering crash dumps, still in use in Lucid & Precise.

Post-Raring installs use kdump-tools, so their related tests are test_kernel_crashdump_kdump() which will test the /var/crash content listed above.

[Martin Pitt (pitti)]

Third time's the charm! The previous patch was failing when running kernel_crashdump with python 3. Thanks to Louis for spotting.

[Martin Pitt (pitti)]

Second patch didn't change in functionality, but needs unfuzzing.

[Marc Deslauriers (mdeslaur)]

this is CVE-2015-1338

[Marc Deslauriers (mdeslaur)]

Patches look good, my only comment would be:

> for dmesg_file in glob.glob(os.path.join(apport.fileutils.report_dir, '*', 'dmesg.*')):

would traverse a symlink, for example:

/var/crash/012345678901 > /root

This doesn't matter when symlink restrictions are on.
When symlink restrictions are disabled, the impact is minor because the glob only looks for dmesg.* files.

I'm not sure if it is worth fixing because of the specific filename. Thoughts?

[Martin Pitt (pitti)]

Thanks Marc, well spotted! I adjusted the first patch to check the owner of the timestamped dir, and add a test case. I also added the CVE to the commit log and NEWS entry.

[Martin Pitt (pitti)]

The second patch didn't change logic, just adds the CVE to NEWS/commit log and unfuzzes NEWS against the previous patch.

[Marc Deslauriers (mdeslaur)]

Updated patches look good, thanks!

[Martin Pitt (pitti)]

Vivid debdiff with both backported patches. It builds, but I don't currently have a vivid VM handy for testing (travelling/low bandwidth). But I ran the hooks and fileutils tests as normal user and root, with py 2 and py3, so should be fine.

[Martin Pitt (pitti)]

debdiff with patches backported to trusty. I ran all the affected test cases manually (in the same scenarios), but wasn't yet able to sbuild/adt-run this.

[Martin Pitt (pitti)]

There was one additional problem which got only exhibited in the autopkgtest, as that runs against the installed binaries: When encountering dangerous log symlinks or dir permissions, kernel_crashdump itself crashed with an OSError. This doesn't matter when running the tests from the checkout, but when running against the installed package you'll get an apport crash report for kernel_crashdump itself (for that OSError). This fails the tests and also is not something which we want to report as a bug in the first place.

This updated patch intercepts the error and just prints a fatal error instead.

[Martin Pitt (pitti)]

Second trunk patch, adjusted to the updated previous one (no functional change).

[Martin Pitt (pitti)]

Updated vivid debdiff. I ran this through the full adt-run testsuite, and it succeeds.

[Martin Pitt (pitti)]

Updated trusty debdiff. Its autopkgtest shows the same (unrelated) failures as the current versions on http://autopkgtest.ubuntu.com/packages/a/apport/trusty/amd64/ .

[Martin Pitt (pitti)]

debdiff with backported patches for precise. This is a bit simpler as precise didn't yet support the kdump mode, and I dropped the python 3 bits. I added a fix for debian/tests so that the test suite actually runs. There are a lot of failures, but this makes it much easier to check/run/evaluate particular tests.

[Marc Deslauriers (mdeslaur)]

FYI, if testing goes well, plan on making this issue public and publishing updates on 2015-09-24.

[Marc Deslauriers (mdeslaur)]

s/plan/I plan/

[Launchpad Janitor (janitor)]

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

---------------
apport (2.0.1-0ubuntu17.10) precise-security; urgency=medium

  * SECURITY FIX: kernel_crashdump: Enforce that the log/dmesg files are not a
    symlink.
    This prevents normal users from pre-creating a symlink to the predictable
    .crash file, and thus triggering a "fill up disk" DoS attack when the
    .crash report tries to include itself. Thanks to halfdog for discovering
    this! (CVE-2015-1338, part of LP #1492570)
  * SECURITY FIX: Fix all writers of report files to open the report file
    exclusively.
    Fix package_hook, kernel_crashdump, and similar hooks to fail if the
    report already exists. This prevents privilege escalation through symlink
    attacks. Note that this will also prevent overwriting previous reports
    with the same same. Thanks to halfdog for discovering this!
    (CVE-2015-1338, LP: #1492570)
  * debian/tests/upstream-system: Change directory to /tmp, so that tests
    actually run against the installed package.

 -- Martin Pitt <email address hidden> Mon, 21 Sep 2015 11:58:45 +0200

[Launchpad Janitor (janitor)]

This bug was fixed in the package apport - 2.17.2-0ubuntu1.5

---------------
apport (2.17.2-0ubuntu1.5) vivid-security; urgency=medium

  * SECURITY FIX: kernel_crashdump: Enforce that the log/dmesg files are not a
    symlink.
    This prevents normal users from pre-creating a symlink to the predictable
    .crash file, and thus triggering a "fill up disk" DoS attack when the
    .crash report tries to include itself. Also clean up the code to make this
    easier to read: Drop the "vmcore_root" alias, move the vmcore and
    vmcore.log cleanup into the "no kdump" section, and replace the buggy
    os.walk() loop with a glob to only catch direct timestamp subdirectories
    of /var/crash/.
    Thanks to halfdog for discovering this!
    (CVE-2015-1338, part of LP #1492570)
  * SECURITY FIX: Fix all writers of report files to open the report file
    exclusively.
    Fix package_hook, kernel_crashdump, and similar hooks to fail if the
    report already exists. This prevents privilege escalation through symlink
    attacks. Note that this will also prevent overwriting previous reports
    with the same same. Thanks to halfdog for discovering this!
    (CVE-2015-1338, LP: #1492570)

 -- Martin Pitt <email address hidden> Mon, 21 Sep 2015 10:22:50 +0200

[Launchpad Janitor (janitor)]

This bug was fixed in the package apport - 2.14.1-0ubuntu3.15

---------------
apport (2.14.1-0ubuntu3.15) trusty-security; urgency=medium

  [ Martin Pitt ]
  * SECURITY FIX: kernel_crashdump: Enforce that the log/dmesg files are not a
    symlink.
    This prevents normal users from pre-creating a symlink to the predictable
    .crash file, and thus triggering a "fill up disk" DoS attack when the
    .crash report tries to include itself. Also clean up the code to make this
    easier to read: Drop the "vmcore_root" alias, move the vmcore and
    vmcore.log cleanup into the "no kdump" section, and replace the buggy
    os.walk() loop with a glob to only catch direct timestamp subdirectories
    of /var/crash/.
    Thanks to halfdog for discovering this!
    (CVE-2015-1338, part of LP #1492570)
  * SECURITY FIX: Fix all writers of report files to open the report file
    exclusively.
    Fix package_hook, kernel_crashdump, and similar hooks to fail if the
    report already exists. This prevents privilege escalation through symlink
    attacks. Note that this will also prevent overwriting previous reports
    with the same same. Thanks to halfdog for discovering this!
    (CVE-2015-1338, LP: #1492570)

  [ Marc Deslauriers ]
  * This package does _not_ contain the changes from 2.14.1-0ubuntu3.14 in
    trusty-proposed.

 -- Marc Deslauriers <email address hidden> Wed, 23 Sep 2015 11:28:26 -0400

[Martin Pitt (pitti)]

I pushed the fixes to trunk. Will do a new release and upload to wily after beta freeze.

[Martin Pitt (pitti)]

New upstream release that contains the fixes: https://launchpad.net/apport/trunk/2.19

[Launchpad Janitor (janitor)]

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

---------------
apport (2.19-0ubuntu1) wily; urgency=medium

  * New upstream release:
    - apport: Drop re-nicing. This might decrease the time a user has to wait
      for apport to finish the core dump for a crashed/hanging foreground
      process. (See LP #1278780)
    - kernel_crashdump: Enforce that the log/dmesg files are not a symlink.
      This prevents normal users from pre-creating a symlink to the
      predictable .crash file, and thus triggering a "fill up disk" DoS attack
      when the .crash report tries to include itself. Thanks to halfdog for
      discovering this! (CVE-2015-1338, part of LP #1492570)
    - SECURITY FIX: Fix all writers of report files (package_hook,
      kernel_crashdump, and similar) to open the report file exclusively,
      i. e. fail if they already exist. This prevents privilege escalation
      through symlink attacks. Note that this will also prevent overwriting
      previous reports with the same same. Thanks to halfdog for discovering
      this! (CVE-2015-1338, LP: #1492570)
    - apport: Ignore process restarts from systemd's watchdog. Their traces
      are usually useless as they don't have any information about the actual
      reasaon why processes hang (like VM suspends or kernel lockups with bad
      hardware) (LP: #1433320)

 -- Martin Pitt <email address hidden> Thu, 24 Sep 2015 14:41:54 +0200