From: Todd C. Miller Date: Sun, 20 Feb 2005 16:48:05 +0000 (+0000) Subject: Just clean the environment once. This assumes that any further X-Git-Tag: SUDO_1_7_0~696 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6bee8e377019c1de4a64f27ffab96e3301551aac;p=sudo Just clean the environment once. This assumes that any further setenv/putenv will be able to handle the fact that we replaced environ with our own malloc'd copy but all the implementations I've checked do. --- diff --git a/env.c b/env.c index a2a8ede56..a42a9f4ca 100644 --- a/env.c +++ b/env.c @@ -78,11 +78,10 @@ struct environment { /* * Prototypes */ -char **rebuild_env __P((char **, char **, int, int)); +char **rebuild_env __P((char **, int, int)); char **zero_env __P((char **)); static void insert_env __P((char *, struct environment *, int)); static char *format_env __P((char *, ...)); -static int var_ok __P((char *)); /* * Default table of "bad" variables to remove from the environment. @@ -150,38 +149,6 @@ static const char *initial_keepenv_table[] = { NULL }; -/* - * Remove potentially dangerous variables from the environment - * and returns a vector of what was pruned out. - */ -char ** -clean_env(envp) - char **envp; -{ - char **ep, **end; - struct environment pruned_env; - - /* Find the end of the environment. */ - for (end = envp; *end; end++) - continue; - - /* - * Prune out any bad environment variables and set user_path, user_prompt - * and prev_user. - */ - memset(&pruned_env, 0, sizeof(pruned_env)); - for (ep = envp; *ep; ) { - if (var_ok(*ep)) { - ep++; - } else { - insert_env(*ep, &pruned_env, 0); - memmove(ep, ep + 1, end - ep); - } - } - - return (pruned_env.envp); -} - /* * Given a variable and value, allocate and format an environment string. */ @@ -267,150 +234,90 @@ insert_env(str, e, dupcheck) *nep = NULL; } -/* - * Check an environment variable against the env_delete - * and env_chec lists. Returns TRUE if allowed, else FALSE. - */ -static int -var_ok(var) - char *var; -{ - struct list_member *cur; - size_t len; - char *cp; - int iswild, okvar = TRUE; - - /* Skip variables with values beginning with () (bash functions) */ - if ((cp = strchr(var, '=')) != NULL) { - if (strncmp(cp, "=() ", 3) == 0) - return (FALSE); - } - - /* Skip anything listed in env_delete. */ - for (cur = def_env_delete; cur && okvar; cur = cur->next) { - len = strlen(cur->value); - /* Deal with '*' wildcard */ - if (cur->value[len - 1] == '*') { - len--; - iswild = TRUE; - } else - iswild = FALSE; - if (strncmp(cur->value, var, len) == 0 && - (iswild || var[len] == '=')) { - okvar = FALSE; - } - } - - /* Check certain variables for '%' and '/' characters. */ - for (cur = def_env_check; cur && okvar; cur = cur->next) { - len = strlen(cur->value); - /* Deal with '*' wildcard */ - if (cur->value[len - 1] == '*') { - len--; - iswild = TRUE; - } else - iswild = FALSE; - if (strncmp(cur->value, var, len) == 0 && - (iswild || var[len] == '=') && - strpbrk(var, "/%")) { - okvar = FALSE; - } - } - - return (okvar); -} - /* * Build a new environment and ether clear potentially dangerous * variables from the old one or start with a clean slate. * Also adds sudo-specific variables (SUDO_*). */ char ** -rebuild_env(envp1, envp2, sudo_mode, noexec) - char **envp1; - char **envp2; +rebuild_env(envp, sudo_mode, noexec) + char **envp; int sudo_mode; int noexec; { + struct list_member *cur; struct environment env; - char **envpv[2], **ep, *cp, *ps1; - int didvar, i; + size_t len; + char **ep, *cp, *ps1; + int okvar, iswild, didvar; /* * Either clean out the environment or reset to a safe default. */ ps1 = NULL; didvar = 0; - envpv[0] = envp1; - envpv[1] = envp2; memset(&env, 0, sizeof(env)); if (def_env_reset) { - struct list_member *cur; - size_t len; - int iswild, keepit; + int keepit; /* Pull in vars we want to keep from the old environment. */ - for (i = 0; i < 2; i++) { - if ((ep = envpv[i]) == NULL) - continue; - for (; *ep; ep++) { - keepit = 0; - - /* Skip variables with values beginning with () (bash functions) */ - if ((cp = strchr(*ep, '=')) != NULL) { - if (strncmp(cp, "=() ", 3) == 0) - continue; - } + for (ep = envp; *ep; ep++) { + keepit = FALSE; - for (cur = def_env_keep; cur; cur = cur->next) { - len = strlen(cur->value); - /* Deal with '*' wildcard */ - if (cur->value[len - 1] == '*') { - len--; - iswild = 1; - } else - iswild = 0; - if (strncmp(cur->value, *ep, len) == 0 && - (iswild || (*ep)[len] == '=')) { - keepit = 1; - break; - } + /* Skip variables with values beginning with () (bash functions) */ + if ((cp = strchr(*ep, '=')) != NULL) { + if (strncmp(cp, "=() ", 3) == 0) + continue; + } + + for (cur = def_env_keep; cur; cur = cur->next) { + len = strlen(cur->value); + /* Deal with '*' wildcard */ + if (cur->value[len - 1] == '*') { + len--; + iswild = TRUE; + } else + iswild = FALSE; + if (strncmp(cur->value, *ep, len) == 0 && + (iswild || (*ep)[len] == '=')) { + keepit = TRUE; + break; } + } - /* For SUDO_PS1 -> PS1 conversion. */ - if (strncmp(*ep, "SUDO_PS1=", 8) == 0) - ps1 = *ep + 5; + /* For SUDO_PS1 -> PS1 conversion. */ + if (strncmp(*ep, "SUDO_PS1=", 8) == 0) + ps1 = *ep + 5; - if (keepit) { - /* Preserve variable. */ - switch (**ep) { - case 'H': - if (strncmp(*ep, "HOME=", 5) == 0) - SET(didvar, DID_HOME); - break; - case 'L': - if (strncmp(*ep, "LOGNAME=", 8) == 0) - SET(didvar, DID_LOGNAME); - break; - case 'P': - if (strncmp(*ep, "PATH=", 5) == 0) - SET(didvar, DID_PATH); - break; - case 'S': - if (strncmp(*ep, "SHELL=", 6) == 0) - SET(didvar, DID_SHELL); - break; - case 'T': - if (strncmp(*ep, "TERM=", 5) == 0) - SET(didvar, DID_TERM); - break; - case 'U': - if (strncmp(*ep, "USER=", 5) == 0) - SET(didvar, DID_USER); - break; - } - insert_env(*ep, &env, 0); + if (keepit) { + /* Preserve variable. */ + switch (**ep) { + case 'H': + if (strncmp(*ep, "HOME=", 5) == 0) + SET(didvar, DID_HOME); + break; + case 'L': + if (strncmp(*ep, "LOGNAME=", 8) == 0) + SET(didvar, DID_LOGNAME); + break; + case 'P': + if (strncmp(*ep, "PATH=", 5) == 0) + SET(didvar, DID_PATH); + break; + case 'S': + if (strncmp(*ep, "SHELL=", 6) == 0) + SET(didvar, DID_SHELL); + break; + case 'T': + if (strncmp(*ep, "TERM=", 5) == 0) + SET(didvar, DID_TERM); + break; + case 'U': + if (strncmp(*ep, "USER=", 5) == 0) + SET(didvar, DID_USER); + break; } + insert_env(*ep, &env, 0); } } @@ -444,20 +351,55 @@ rebuild_env(envp1, envp2, sudo_mode, noexec) * Copy envp entries as long as they don't match env_delete or * env_check. */ - for (i = 0; i < 2; i++) { - if ((ep = envpv[i]) == NULL) - continue; - for (; *ep; ep++) { - if (var_ok(*ep)) { - if (strncmp(*ep, "SUDO_PS1=", 9) == 0) - ps1 = *ep + 5; - else if (strncmp(*ep, "PATH=", 5) == 0) - SET(didvar, DID_PATH); - else if (strncmp(*ep, "TERM=", 5) == 0) - SET(didvar, DID_TERM); - insert_env(*ep, &env, 0); + for (ep = envp; *ep; ep++) { + okvar = TRUE; + + /* Skip variables with values beginning with () (bash functions) */ + if ((cp = strchr(*ep, '=')) != NULL) { + if (strncmp(cp, "=() ", 3) == 0) + continue; + } + + /* Skip anything listed in env_delete. */ + for (cur = def_env_delete; cur && okvar; cur = cur->next) { + len = strlen(cur->value); + /* Deal with '*' wildcard */ + if (cur->value[len - 1] == '*') { + len--; + iswild = TRUE; + } else + iswild = FALSE; + if (strncmp(cur->value, *ep, len) == 0 && + (iswild || (*ep)[len] == '=')) { + okvar = FALSE; + } + } + + /* Check certain variables for '%' and '/' characters. */ + for (cur = def_env_check; cur && okvar; cur = cur->next) { + len = strlen(cur->value); + /* Deal with '*' wildcard */ + if (cur->value[len - 1] == '*') { + len--; + iswild = TRUE; + } else + iswild = FALSE; + if (strncmp(cur->value, *ep, len) == 0 && + (iswild || (*ep)[len] == '=') && + strpbrk(*ep, "/%")) { + okvar = FALSE; } } + + if (okvar) { + if (strncmp(*ep, "SUDO_PS1=", 9) == 0) + ps1 = *ep + 5; + else if (strncmp(*ep, "PATH=", 5) == 0) + SET(didvar, DID_PATH); + else if (strncmp(*ep, "TERM=", 5) == 0) + SET(didvar, DID_TERM); + insert_env(*ep, &env, 0); + } } } /* Replace the PATH envariable with a secure one? */ diff --git a/sudo.c b/sudo.c index 0416b7b62..b4e9e1949 100644 --- a/sudo.c +++ b/sudo.c @@ -110,7 +110,7 @@ static void usage_excl __P((int)) __attribute__((__noreturn__)); static struct passwd *get_authpw __P((void)); extern int sudo_edit __P((int, char **)); -extern char **rebuild_env __P((char **, char **, int, int)); +extern char **rebuild_env __P((char **, int, int)); extern char **clean_env __P((char **)); /* @@ -151,7 +151,6 @@ main(argc, argv) int cmnd_status; int sudo_mode; int pwflag; - char **new_environ, **pruned_environ; sigaction_t sa; #ifdef HAVE_LDAP VOID *ld; @@ -292,8 +291,6 @@ main(argc, argv) def_closefrom = user_closefrom; } - pruned_environ = clean_env(environ); - cmnd_status = set_cmnd(sudo_mode); #ifdef HAVE_LDAP @@ -360,9 +357,7 @@ main(argc, argv) /* Build a new environment based on the rules in sudoers. */ if (ISSET(sudo_mode, MODE_RUN)) - new_environ = rebuild_env(pruned_environ, environ, sudo_mode, def_noexec); - else - new_environ = environ; + environ = rebuild_env(environ, sudo_mode, def_noexec); if (ISSET(validated, VALIDATE_OK)) { /* Finally tell the user if the command did not exist. */ @@ -438,7 +433,7 @@ main(argc, argv) if (ISSET(sudo_mode, MODE_BACKGROUND) && fork() > 0) exit(0); else - execve(safe_cmnd, NewArgv, new_environ); + execve(safe_cmnd, NewArgv, environ); #else exit(0); #endif /* PROFILING */ @@ -449,7 +444,7 @@ main(argc, argv) NewArgv--; /* at least one extra slot... */ NewArgv[0] = "sh"; NewArgv[1] = safe_cmnd; - execve(_PATH_BSHELL, NewArgv, new_environ); + execve(_PATH_BSHELL, NewArgv, environ); } warning("unable to execute %s", safe_cmnd); exit(127);