Calibre and setuid

Programs that regular users can run with root privileges (i.e. setuid programs) are tricky to write correctly, which is why they are best avoided whenever possible. But there are, of course, things that users need to be able to do that require privileges (changing passwords is a canonical example), which is why the setuid/setgid facility exists in Unix. When the need arises, however, a great deal of care should be taken before writing and releasing a program that is meant to be installed as setuid (aka SUID). A recent bug report for the Calibre e-book reader shows the kinds of problems that can come about if proper care is not exercised.

Jason Donenfeld reported several bugs in the setuid calibre-mount-helper program to the project's Launchpad bug tracker on November 1. Essentially, the program is meant to allow Calibre users to mount various removable devices (like e-book readers connected via USB) to update the content on readers. Unfortunately, it suffered from five separate problems that Donenfeld reported, some of which could be easily used as a local privilege escalation—to root.

The problems themselves read like a laundry list of things not to do in a setuid program including not sanitizing user input, not setting the PATH environment variable before calling exec(), and not sanity checking arguments. It's not completely surprising that Calibre got these things wrong, as they are fairly common programming mistakes. For programs running with a user's privileges, those kinds of errors typically just lead to bugs of various sorts (some of which could have fairly catastrophic consequences for the user, but not the system as a whole). For setuid programs, though, errors like those can lead to very serious security holes—as they did here.

Something that was a bit surprising was the combative tone that Calibre's lead developer Kovid Goyal took in the comments on the bug. Rather than working with Donenfeld to see what the problems were, he dismissed most of the bugs as invalid. Even after Donenfeld tried to further point out the problems, Goyal was rather sarcastic in response:

The proof-of-concept that Donenfeld posted with the bug—fairly snarky in tone, which may not have helped—clearly showed how to exploit the lack of PATH sanitization to get root privileges, but didn't demonstrate a different reported problem with argument injection to the mount program. That particular problem results from not sanitizing the user input and passing it on to mount. The other three problems (arbitrary root-owned directory creation, arbitrary empty directory removal, and creating .created_by_calibre_mount_helper files anywhere in the filesystem) are going to be harder to exploit, but they certainly aren't capabilities that regular users should have.

Goyal fixed the PATH issue immediately, but didn't see the argument injection problem, and therefore didn't fix it. Dan Rosenberg pointed out that there was still a bug:

In addition, Rosenberg suggested that there were safer ways to allow users to mount removable media, rather than writing a setuid application specific to Calibre. Once again, though, Goyal takes the combative approach. He committed a fix (though it doesn't really solve the problem) for the problem of "mounting on top of important directories", but seems affronted by the bug comments:

Part of the problem is that Goyal is looking for a single solution to the mounting problem that works "on *every single linux distro that the current technique works on* not just version >= x of distro y". While that complaint has some merit, it is hardly an excuse to introduce security holes into the system. Though Goyal claims to be aware of the "dangers" of setuid programs, the code in calibre-mount-helper does not really bear that out.

In fact, Donenfeld quickly came back with an example exploit that routed around Goyal's fix. The fix just disallows mounting in /usr, /bin, or /sbin, but Donenfeld's example mounts a /etc filesystem (with a chosen root password in passwd), thus allowing the user to log in as root. Obviously, /etc can be added to the disallowed list, but that becomes something of an arms race. A whitelisting approach might be more reasonable, but a better solution would be to use the distribution-supplied mechanisms for mounting the e-book readers. Those solutions should have had most of the obvious (and some non-obvious) problems shaken out, though there is no universal cross-distribution mechanism as Goyal would like to see.

As Donenfeld points out, Debian does not install the mount helper, and instead uses a wrapper script around udisks. Fedora also avoids the mount helper. Judging from this bug report, Ubuntu has picked up the Debian fix as well. It is unclear whether any of those distributions made an effort to get the word out about the problem or get a fix upstream. openSUSE seems to install calibre-mount-helper, however, and various other distributions may as well. In any case, anyone who picks up the source package and installs it will get the program installed as a setuid binary in /opt/calibre/bin.

Writing secure code is hard. Programmers tend to focus on what they are trying to accomplish, rather than all of the different ways the program can be abused. That's not an excuse, but it is an explanation of sorts. Distributions and users should be especially vigilant about setuid programs that come in from packages that, arguably anyway, shouldn't need them. Projects should probably also try to engage with folks that report security problems, rather than attacking them.

As of this writing, the Calibre trunk is still vulnerable to the example exploit that Donenfeld posted. One would expect to see a fix for it soon, and that any distributions that install calibre-mount-helper to issue updates. Users that have it installed from source may want to investigate using a wrapper script or other means to disarm the bug until the fix is made, at least on shared machines.