Overhaul of environment handling. Fix daft bug in path search code.
authormdw <mdw>
Wed, 20 Aug 1997 16:15:13 +0000 (16:15 +0000)
committermdw <mdw>
Wed, 20 Aug 1997 16:15:13 +0000 (16:15 +0000)
src/become.c

index a690072..6f5c236 100644 (file)
@@ -1,6 +1,6 @@
 /* -*-c-*-
  *
- * $Id: become.c,v 1.3 1997/08/07 16:28:59 mdw Exp $
+ * $Id: become.c,v 1.4 1997/08/20 16:15:13 mdw Exp $
  *
  * Main code for `become'
  *
@@ -29,6 +29,9 @@
 /*----- Revision history --------------------------------------------------*
  *
  * $Log: become.c,v $
+ * Revision 1.4  1997/08/20 16:15:13  mdw
+ * Overhaul of environment handling.  Fix daft bug in path search code.
+ *
  * Revision 1.3  1997/08/07 16:28:59  mdw
  * Do something useful when users attempt to become themselves.
  *
@@ -105,7 +108,7 @@ static void bc__write(FILE *fp, const char *p)
 
   /* --- Try to be a little efficient --- *
    *
-   * Gather up non-`$' characters using @strcpn@ and spew them out really
+   * Gather up non-`$' characters using @strcspn@ and spew them out really
    * quickly.
    */
 
@@ -174,13 +177,14 @@ static char *bc__makeEnv(const char *var, const char *value)
 /* --- @bc__help@ --- *
  *
  * Arguments:  @FILE *fp@ = stream to write on
+ *             @int suid@ = whether we're running set-uid
  *
  * Returns:    ---
  *
  * Use:                Displays a help message for this excellent piece of software.
  */
 
-static void bc__help(FILE *fp)
+static void bc__help(FILE *fp, int suid)
 {
   bc__banner(fp);
   putc('\n', fp);
@@ -206,13 +210,16 @@ static void bc__help(FILE *fp)
 "-c, --command=CMD     Run the (Bourne) shell command CMD\n"
 "-d, --daemon          Start up a daemon, to accept requests from clients\n"
 "-p, --port=PORT               In daemon mode, listen on PORT\n"
-"-f, --config-file=FILE        In daemon mode, read config from FILE\n"
+"-f, --config-file=FILE        In daemon mode, read config from FILE\n");
 #ifdef TRACING
-"--impersonate=USER    Claim to be USER when asking the server\n"
+  if (!suid) {
+    bc__write(fp,
+"--impersonate=USER    Claim to be USER when asking the server\n");
+  }
+  bc__write(fp,
 "--trace=FILE          Dump trace information to FILE (boring)\n"
-"--trace-level=OPTS    Set level of tracing information\n"
+"--trace-level=OPTS    Set level of tracing information\n");
 #endif
-);
 }
 
 /* --- @main@ --- *
@@ -255,16 +262,6 @@ int main(int argc, char *argv[])
     0                                  /* Terminator */
   };
 
-  /* --- Login request replaces the environment with this --- */
-
-  static char *mangled_env[] = {
-    "PATH=/bin:/usr/bin",              /* Default `clean' path*/
-    0,                                 /* User's name (`USER') */
-    0,                                 /* User's name (`LOGNAME') */
-    0,                                 /* Home directory (`HOME') */
-    0                                  /* Terminator */
-  };
-
   /* --- Definitions for the various flags --- */
 
   enum {
@@ -288,6 +285,7 @@ int main(int argc, char *argv[])
     int i;
     static struct option opts[] = {
       { "help",                0,              0,      'h' },
+      { "usage",       0,              0,      'U' },
       { "version",     0,              0,      'v' },
       { "login",       0,              0,      'l' },
       { "command",     gFlag_argReq,   0,      'c' },
@@ -311,7 +309,11 @@ int main(int argc, char *argv[])
       /* --- Standard help and version options --- */
 
       case 'h':
-       bc__help(stdout);
+       bc__help(stdout, flags & f_setuid);
+       exit(0);
+       break;
+      case 'U':
+       bc__usage(stdout);
        exit(0);
        break;
       case 'v':
@@ -344,6 +346,7 @@ int main(int argc, char *argv[])
        */
 
 #ifdef TRACING
+
       case 'I':
        if (flags & f_setuid)
          moan("shan't allow impersonation while running setuid");
@@ -360,6 +363,7 @@ int main(int argc, char *argv[])
          flags |= f_dummy;
        }
        break;
+
 #endif
 
       /* --- Tracing support --- *
@@ -573,19 +577,107 @@ int main(int argc, char *argv[])
   /* --- An unadorned becoming requires little work --- */
 
   else {
-    shell[0] = from_pw->pw_shell;
+    shell[0] = getenv("SHELL");
+    if (!shell[0])
+      shell[0] = from_pw->pw_shell;
     shell[1] = 0;
     todo = shell;
     binary = todo[0];
   }
 
-  /* --- Mangle the environment if login flag given --- */
+  /* --- Mangle the environment --- *
+   *
+   * This keeps getting more complicated all the time.
+   */
+
+  {
+    size_t i, j, b;
+    int pass;
+
+    /* --- Expunge some environment variables --- *
+     *
+     * Any environment string which has one of the following as a prefix
+     * will be expunged from the environment passed to the called process.
+     * The first line lists variables which have been used to list search
+     * paths for shared libraries: by manipulating these, an attacker could
+     * replace a standard library with one of his own.  The second line lists
+     * other well-known dangerous environment variables.
+     */
+
+    static const char *banned[] = {
+      "LD_", "SHLIB_PATH=", "LIBPATH=", "_RLD_",
+      "IFS=", "ENV=", "BASH_ENV=", "KRB_CONF=",
+      0
+    };
+
+    /* --- Do this in two passes --- *
+     *
+     * The first pass works out how big the environment block needs to be.
+     * The second actually fills it.
+     */
+
+    for (pass = 0; pass < 2; pass++) {
+      i = j = 0;
 
-  if (flags & f_login) {
-    env = mangled_env;
-    env[1] = bc__makeEnv("USER", to_pw->pw_name);
-    env[2] = bc__makeEnv("LOGNAME", to_pw->pw_name);
-    env[3] = bc__makeEnv("HOME", to_pw->pw_dir);
+      if (flags & f_login) {
+
+       /* --- This is a login request --- *
+        *
+        * Erase the existing environment and build a new one.
+        */
+
+       if (!pass)
+         i += 4;
+       else {
+         env[i++] = "PATH=/usr/bin:/bin";
+         env[i++] = bc__makeEnv("USER", to_pw->pw_name);
+         env[i++] = bc__makeEnv("LOGNAME", to_pw->pw_name);
+         env[i++] = bc__makeEnv("HOME", to_pw->pw_dir);
+       }
+      } else {
+
+       /* --- Normal request --- *
+        *
+        * Remove dangerous variables from the list.
+        */
+
+       for (j = 0; environ[j]; j++) {
+         for (b = 0; banned[b]; b++) {
+           if (memcmp(environ[j], banned[b], strlen(banned[b])) == 0)
+             goto skip_var;
+         }
+         if (pass)
+           env[i] = environ[j];
+         i++;
+       skip_var:;
+       }
+      }
+
+      /* --- Now add our own variables --- *
+       *
+       * The following are supplied only to help people construct startup
+       * scripts.  Anyone who relies on them being accurate for
+       * authentication purposes will get exactly what they deserve.
+       */
+
+      if (!pass)
+       i += 4;
+      else {
+       env[i++] = bc__makeEnv("BECOME_OLDUSER", from_pw->pw_name);
+       env[i++] = bc__makeEnv("BECOME_OLDHOME", from_pw->pw_dir);
+       env[i++] = bc__makeEnv("BECOME_USER", to_pw->pw_name);
+       env[i++] = bc__makeEnv("BECOME_HOME", to_pw->pw_dir);
+      }
+
+      /* --- Allocate memory after the first pass is complete --- */
+
+      if (pass)
+       env[i] = 0;
+      i++;
+
+      if (!pass)
+       env = xmalloc(i * sizeof(env[0]));
+    }
   }
 
   /* --- Trace the command --- */
@@ -612,9 +704,9 @@ int main(int argc, char *argv[])
       p = "/bin:/usr/bin";
     path = xstrdup(p);
 
-    for (p = strtok(path, ":"); (p = strtok(0, ":")) != 0; ) {
+    for (p = strtok(path, ":"); p; p = strtok(0, ":")) {
 
-      /* --- SECURITY: check length of string before copying --- */
+      /* --- Check length of string before copying --- */
 
       if (strlen(p) + strlen(binary) + 2 > sizeof(rq.cmd))
        continue;
@@ -657,7 +749,7 @@ int main(int argc, char *argv[])
 
     while (*q) {
 
-      /* --- SECURITY: check for buffer overflows here --- *
+      /* --- Check for buffer overflows here --- *
        *
        * I write at most one byte per iteration so this is OK.  Remember to
        * allow one for the null byte.