X-Git-Url: https://git.distorted.org.uk/~mdw/become/blobdiff_plain/f175e71b7b9545c410a6de1229b121474e84a834..59f147b0d9bfb46577cd6f8813589ccdf8bc2a2c:/src/become.c

diff --git a/src/become.c b/src/become.c
index 67eec16..23f69dd 100644
--- a/src/become.c
+++ b/src/become.c
@@ -1,6 +1,6 @@
 /* -*-c-*-
  *
- * $Id: become.c,v 1.16 1998/04/23 13:21:04 mdw Exp $
+ * $Id: become.c,v 1.19 1998/06/29 13:10:41 mdw Exp $
  *
  * Main code for `become'
  *
@@ -29,6 +29,22 @@
 /*----- 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.
+ *
+ * Revision 1.17  1998/06/18 15:06:59  mdw
+ * Close log before execing program to avoid leaving a socket open.
+ *
  * Revision 1.16  1998/04/23 13:21:04  mdw
  * Small tweaks.  Support no-network configuration option, and rearrange
  * the help text a little.
@@ -952,7 +968,7 @@ done_options:
     uname(&u);
     if ((he = gethostbyname(u.nodename)) == 0)
       die("who am I? (can't resolve `%s')", u.nodename);
-    memcpy(&rq.host, he->h_addr, sizeof(struct in_addr));
+    memcpy(&rq.host, he->h_addr, sizeof(rq.host));
   }
 
   /* --- Fiddle with group ownerships a bit --- */
@@ -1370,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 */
@@ -1521,6 +1548,7 @@ done_options:
   /* --- Finally, call the program --- */
 
   fflush(0);
+  closelog();
   execve(rq.cmd, todo, env);
   die("couldn't exec `%s': %s", rq.cmd, strerror(errno));
   return (127);