From 569b8891b31062e078bc7c1e25a13e04ddde8e53 Mon Sep 17 00:00:00 2001 From: mdw Date: Wed, 20 Aug 1997 16:15:13 +0000 Subject: [PATCH] Overhaul of environment handling. Fix daft bug in path search code. --- src/become.c | 148 ++++++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 120 insertions(+), 28 deletions(-) diff --git a/src/become.c b/src/become.c index a690072..6f5c236 100644 --- a/src/become.c +++ b/src/become.c @@ -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. -- 2.11.0