]> granicus.if.org Git - sudo/commitdiff
Just clean the environment once. This assumes that any further
authorTodd C. Miller <Todd.Miller@courtesan.com>
Sun, 20 Feb 2005 16:48:05 +0000 (16:48 +0000)
committerTodd C. Miller <Todd.Miller@courtesan.com>
Sun, 20 Feb 2005 16:48:05 +0000 (16:48 +0000)
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.

env.c
sudo.c

diff --git a/env.c b/env.c
index a2a8ede561dd06f1c9e288d72bbd41769c81d2d7..a42a9f4caba9415b64325df0fbb87c933f0f9c44 100644 (file)
--- 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 0416b7b62f4896de1455dc948acadd8d204b5d43..b4e9e19492d7976b7b838596a93c1bd1a53034d5 100644 (file)
--- 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);