[4.3 PATCH] locate fails when run as root

[Nix]

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  <address@hidden>

        * 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. */


-- 
`He accused the FSF of being "something of a hypocrit", which
 shows that he neither understands hypocrisy nor can spell.'
   --- jimmybgood