(I'll try to figure out where the previous mails went to)
---------- Forwarded message ---------- From: Nix <[EMAIL PROTECTED]> Date: Jan 19, 2007 12:06 AM Subject: [4.3 PATCH] locate fails when run as root To: James Youngman <[EMAIL PROTECTED]> [This is a 3rd resend, since repeated attempts to send this to to [email protected] have failed; the email vanished without trace; maybe they're stuck in a moderation queue somewhere, but maybe they're lost. Apologies if they turn up after I send this...] locate's new slocate support has broken locate when run as root, because of an assumption in drop_privs() that failure to drop privileges is caused by attackers running locate without CAP_SETUID. This is obviously not always true: you can't drop privileges if your real uid is already root, and in that situation setuid(0) will obviously still return 0 because we are still root, yet there is no attack. (The code is odd in this area: are there really systems on which setuid() can fail without returning nonzero and setting errno? That seems a gross violation of POSIX to me.) (As an aside, it's strange and rather inefficient that the secure DB is opened even if we later decide that we don't need it, perhaps because it doesn't exist.) Combined with the non-resetting of errno before setuid() is called, this can lead to comical error messages: hades:~# locate Errno.pm locate: Failed to drop privileges: No such file or directory Er, I don't think so :) that was an open() failure for the secure_db! Thus this patch, against findutils-4.3.2, with a free additional gratuitous comment typo fix: 2007-01-15 Nix <[EMAIL PROTECTED]> * locate/locate.c (drop_privs): Don't try to drop privileges if our real uid is root. Index: findutils/locate/locate.c =================================================================== --- findutils.orig/locate/locate.c 2006-12-16 10:46:28.000000000 +0000 +++ findutils/locate/locate.c 2007-01-15 22:59:10.000000000 +0000 @@ -1345,7 +1345,7 @@ const char * what = "failed"; uid_t orig_euid = geteuid(); - /* Use of setgroups() is restrcted to root only. */ + /* Use of setgroups() is restricted to root only. */ if (0 == orig_euid) { gid_t groups[1]; @@ -1356,22 +1356,28 @@ goto fail; } } - - if (0 != setuid(getuid())) + + /* If we are not run with a real uid of root, try to drop our + excess privileges. */ + + if (getuid() != 0) { - what = _("failed to drop setuid privileges"); - goto fail; - } + if (0 != setuid(getuid())) + { + what = _("failed to drop setuid privileges"); + goto fail; + } - /* Defend against the case where the attacker runs us with the - * capability to call setuid() turned off, which on some systems - * will cause the above attempt to drop privileges fail (leaving us - * privileged). - */ - if (0 == setuid(0)) - { - what = _("Failed to drop privileges"); - goto fail; + /* Defend against the case where the attacker runs us with the + * capability to call setuid() turned off, which on some systems + * will cause the above attempt to drop privileges fail (leaving us + * privileged). + */ + if (0 == setuid(0)) + { + what = _("Failed to drop privileges"); + goto fail; + } } /* success. */ -- `The serial comma, however, is correct and proper, and abandoning it will surely lead to chaos, anarchy, rioting in the streets, the Terrorists taking over, and possibly the complete collapse of Human Civilization.' _______________________________________________ Bug-findutils mailing list [email protected] http://lists.gnu.org/mailman/listinfo/bug-findutils
