From: Colin Watson Date: Sun, 11 Dec 2016 23:45:36 +0000 (+0000) Subject: The sad tale of CVE-2015-1336 X-Git-Url: https://www.chiark.greenend.org.uk/ucgi/~cjwatson/git?a=commitdiff_plain;h=dfa80b2b66eeae766121dcc2c4ebd4a9045fcb5d;p=blog.git The sad tale of CVE-2015-1336 --- diff --git a/content/cve-2015-1336.md b/content/cve-2015-1336.md new file mode 100644 index 00000000..1847ff80 --- /dev/null +++ b/content/cve-2015-1336.md @@ -0,0 +1,105 @@ +Title: The sad tale of CVE-2015-1336 +Slug: cve-2015-1336 +Date: 2016-12-11 23:42:55 +00:00 +Category: man-db +Tags: man-db, planet-debian, planet-ubuntu + +Today I released man-db 2.7.6 +([announcement](https://lists.nongnu.org/archive/html/man-db-announce/2016-12/msg00000.html), +[NEWS](http://git.savannah.gnu.org/cgit/man-db.git/tree/NEWS?id=2.7.6), +[git log](http://git.savannah.gnu.org/cgit/man-db.git/log/?h=2.7.6)), and +uploaded it to Debian unstable. The major change in this release was a set +of fixes for two security vulnerabilities, +[one](http://www.halfdog.net/Security/2015/SetgidDirectoryPrivilegeEscalation/) +of which affected all man-db installations since 2.3.12 (or 2.3.10-66 in +Debian), and [the +other](http://www.halfdog.net/Security/2015/MandbSymlinkLocalRootPrivilegeEscalation/) +of which was specific to Debian and its derivatives. + +It's probably obvious from the dates here that this has not been my finest +hour in terms of responding to security issues in a timely fashion, and I +apologise for that. Some of this is just the usual life reasons, which I +shan't bore you by reciting, but some of it has been that fixing this +properly in man-db was genuinely rather complicated and delicate. Since +I've previously advocated man-db over some of its competitors on the basis +of a better security posture, I think it behooves me to write up a longer +description. + +I took over maintaining man-db over fifteen years ago in slightly unexpected +circumstances (I got annoyed with its bug list and made a couple of +non-maintainer uploads, and then the previous maintainer +[died](https://www.debian.org/News/2001/20010402b), so I ended up taking +over both in Debian and upstream). I was a fairly new developer at the +time, and there weren't a lot of people I could ask questions of, but I did +my best to recover as much of the history as I could and learn from it. One +thing that became clear very quickly, both from my own inspection and from +the bug list, was that most of the code had been written in a rather more +innocent time. It was absolutely riddled with dangerous uses of the shell, +poor temporary file handling, buffer overruns, and various common-or-garden +deficiencies of that kind. I spent several years reworking large swathes of +the codebase to be more robust against those kinds of bugs by design, and +for example [libpipeline](http://libpipeline.nongnu.org/) came out of that +effort. + +The most subtle and risky set of problems came from the fact that the `man` +and `mandb` programs were installed set-user-id to the `man` user. Part of +this was so that `man` could maintain preformatted "cat pages", and part of +it was so that users could run `mandb` if the system databases were out of +date (this is now much less useful since most package managers, including +`dpkg`, support some kind of trigger mechanism that can run `mandb` whenever +new system-level manual pages are installed). One of the first things I did +was to make this optional, and this has been a disabled-by-default `debconf` +option in Debian for a long time now. But it's still a supported option and +is enabled by default upstream, and when running setuid `man` and `mandb` +need to take care to drop privileges when dealing with user-controlled data +and to write files with the appropriate ownership and permissions. + +My predecessor had problems related to this such as +[Debian #26002](https://bugs.debian.org/26002), and one of the ways they +dealt with them was to make `/var/cache/man/` set-group-id root, in order +that files written to that directory would have consistent group ownership. +This always struck me as rather strange and I meant to do something about it +at some point, but until the first vulnerability report above I regarded it +as mainly a curiosity, since nothing in there was group-writeable anyway. +As a result, with the more immediate aim of making the system behave +consistently and dealing with bug reports, various bits of code had accreted +that assumed that `/var/cache/man/` would be `man:root 2755`, and not all of +it was immediately obvious. + +This interacted with the second vulnerability report in two ways. Firstly, +at some level it caused it because I was dealing with the day-to-day +problems rather than thinking at a higher level: a +[series](https://bugs.debian.org/129340) +[of](https://bugs.debian.org/619726) [bugs](https://bugs.debian.org/734063) +led me down the path of whacking problems over the head with a recursive +`chown` of `/var/cache/man/` from `cron`, rather than working out why things +got that way in the first place. Secondly, once I'd done that, I couldn't +remove the `chown` without a much more extensive excursion into all the code +that dealt with cache files, for fear of reintroducing those bugs. So +although the fix for the second vulnerability is [very simple in +itself](https://anonscm.debian.org/cgit/pkg-man-db/man-db.git/commit/?id=2f47ed4e682183f60f9aeed7f69f61e162019b20), +I couldn't get there without dealing with the first vulnerability. + +In some ways, of course, cat pages are a bit of an anachronism. Most modern +systems can format pages quickly enough that it's not much of an issue. +However, I'm loath to drop the feature entirely: I'm generally wary of +assuming that just because I have a fast system that everyone does. So, +instead, I +[did](http://git.savannah.gnu.org/cgit/man-db.git/commit/?id=31552334cecee82809059ec598a37d9ea82683f0) +what I should have done years ago: make `man` and `mandb` set-group-id `man` +as well as set-user-id `man`, at which point we can simply make all the +cache files and directories be owned by `man:man` and drop the setgid bit on +cache directories. This should be simpler and less prone to +difficult-to-understand problems. + +I expect that my next substantial upstream release will switch to +`--disable-setuid` by default to reduce exposure, though, and distributions +can start thinking about whether they want to follow that (Fedora already +does, for example). If this becomes widely disabled without complaints then +that would be good evidence that it's reasonable to drop the feature +entirely. I'm not in a rush, but if you do need cat pages then now is a +good time to write to me and tell me why. + +This is the fiddliest set of vulnerabilities I've dealt with in man-db for +quite some time, so I hope that if there are more then I can get back to my +previous quick response time.