Add some commentary regarding an issue in `sudo' which affects `become';

I'm not fixing it yet because I don't think it's important.

Fixed the PATH lookup code to use the right binary name: `binary' rather
than `todo[0]'.  The two only differ when `style' is `l_login', in which
case `binary' has an initial `/' anyway, and the PATH lookup code is
never invoked.  The name is used in a buffer-overflow precheck, though,
and auditing is easier if the naming is consistent.

diff --git a/src/become.c b/src/become.c
index c5df906..23f69dd 100644 (file)
--- a/src/become.c
+++ b/src/become.c
@@ -1,6 +1,6 @@
 /* -*-c-*-
  *
- * $Id: become.c,v 1.18 1998/06/26 10:32:54 mdw Exp $
+ * $Id: become.c,v 1.19 1998/06/29 13:10:41 mdw Exp $
  *
  * Main code for `become'
  *
@@ -29,6 +29,16 @@
 /*----- Revision history --------------------------------------------------*
  *
  * $Log: become.c,v $
+ * Revision 1.19  1998/06/29 13:10:41  mdw
+ * Add some commentary regarding an issue in `sudo' which affects `become';
+ * I'm not fixing it yet because I don't think it's important.
+ *
+ * Fixed the PATH lookup code to use the right binary name: `binary' rather
+ * than `todo[0]'.  The two only differ when `style' is `l_login', in which
+ * case `binary' has an initial `/' anyway, and the PATH lookup code is
+ * never invoked.  The name is used in a buffer-overflow precheck, though,
+ * and auditing is easier if the naming is consistent.
+ *
  * Revision 1.18  1998/06/26 10:32:54  mdw
  * Cosmetic change: use sizeof(destination) in memcpy.
  *
@@ -1376,9 +1386,20 @@ done_options:
       if (strlen(p) + strlen(binary) + 2 > sizeof(rq.cmd))
        continue;
 
-      /* --- Now build the pathname and check it --- */
+      /* --- Now build the pathname and check it --- *
+       *
+       * Issue: user can take advantage of these privileges to decide whether
+       * a program with a given name exists.  I'm not sure that's
+       * particularly significant: it only works on regular files with
+       * execute permissions, and if you're relying on the names of these
+       * being secret to keep your security up, then you're doing something
+       * deeply wrong anyway.  On the other hand, it's useful to allow people
+       * to be able to execute programs and scripts which they wouldn't
+       * otherwise have access to.  [This problem was brought up on
+       * Bugtraq, as a complaint against sudo.]
+       */
 
-      sprintf(rq.cmd, "%s/%s", p, todo[0]);
+      sprintf(rq.cmd, "%s/%s", p, binary);
       if (stat(rq.cmd, &st) == 0 &&    /* Check it exists */
          st.st_mode & 0111 &&          /* Check it's executable */
          S_ISREG(st.st_mode))          /* Check it's a file */