From 76ddc2e3e4837d0557fb6626394f87648e5f8c3b Mon Sep 17 00:00:00 2001 From: Denis Vlasenko Date: Tue, 30 Dec 2008 05:05:31 +0000 Subject: [PATCH] libbb: add bb_unsetenv (taken from hush). udhcpc: stop filtering environment passed to the script. crond: fix uncovered potential bug (failing unsetenv) mdev: fix uncovered potential bug (failing unsetenv) tcp, udpsvd: fix uncovered potential bug (failing unsetenv) function old new delta safe_setenv - 58 +58 bb_unsetenv - 55 +55 builtin_unset 139 138 -1 tcpudpsvd_main 1843 1830 -13 free_strings_and_unsetenv 87 53 -34 udhcp_run_script 1186 1133 -53 safe_setenv4 62 - -62 ------------------------------------------------------------------------------ (add/remove: 2/1 grow/shrink: 0/4 up/down: 113/-163) Total: -50 bytes --- include/libbb.h | 1 + libbb/xfuncs_printf.c | 23 ++++++++++++++ miscutils/crond.c | 12 +++---- networking/tcpudp.c | 3 +- networking/udhcp/script.c | 66 ++++++++++++++++++--------------------- shell/hush.c | 14 ++------- util-linux/mdev.c | 3 +- 7 files changed, 65 insertions(+), 57 deletions(-) diff --git a/include/libbb.h b/include/libbb.h index e0541a731..e1a6d120b 100644 --- a/include/libbb.h +++ b/include/libbb.h @@ -377,6 +377,7 @@ void xsetuid(uid_t uid) FAST_FUNC; void xchdir(const char *path) FAST_FUNC; void xchroot(const char *path) FAST_FUNC; void xsetenv(const char *key, const char *value) FAST_FUNC; +void bb_unsetenv(const char *key) FAST_FUNC; void xunlink(const char *pathname) FAST_FUNC; void xstat(const char *pathname, struct stat *buf) FAST_FUNC; int xopen(const char *pathname, int flags) FAST_FUNC FAST_FUNC; diff --git a/libbb/xfuncs_printf.c b/libbb/xfuncs_printf.c index 108e14043..46ae7ac60 100644 --- a/libbb/xfuncs_printf.c +++ b/libbb/xfuncs_printf.c @@ -333,6 +333,29 @@ void FAST_FUNC xsetenv(const char *key, const char *value) bb_error_msg_and_die(bb_msg_memory_exhausted); } +/* Handles "VAR=VAL" strings, even those which are part of environ + * _right now_ + */ +void FAST_FUNC bb_unsetenv(const char *var) +{ + char *tp = strchr(var, '='); + + if (!tp) { + unsetenv(var); + return; + } + + /* In case var was putenv'ed, we can't replace '=' + * with NUL and unsetenv(var) - it won't work, + * env is modified by the replacement, unsetenv + * sees "VAR" instead of "VAR=VAL" and does not remove it! + * horror :( */ + tp = xstrndup(var, tp - var); + unsetenv(tp); + free(tp); +} + + // Die with an error message if we can't set gid. (Because resource limits may // limit this user to a given number of processes, and if that fills up the // setgid() will fail and we'll _still_be_root_, which is bad.) diff --git a/miscutils/crond.c b/miscutils/crond.c index 732fbb147..12560fa36 100644 --- a/miscutils/crond.c +++ b/miscutils/crond.c @@ -252,14 +252,12 @@ int crond_main(int argc UNUSED_PARAM, char **argv) /* We set environment *before* vfork (because we want to use vfork), * so we cannot use setenv() - repeated calls to setenv() may leak memory! * Using putenv(), and freeing memory after unsetenv() won't leak */ -static void safe_setenv4(char **pvar_val, const char *var, const char *val /*, int len*/) +static void safe_setenv(char **pvar_val, const char *var, const char *val) { - const int len = 4; /* both var names are 4 char long */ char *var_val = *pvar_val; if (var_val) { - var_val[len] = '\0'; /* nuke '=' */ - unsetenv(var_val); + bb_unsetenv(var_val); free(var_val); } *pvar_val = xasprintf("%s=%s", var, val); @@ -270,10 +268,10 @@ static void safe_setenv4(char **pvar_val, const char *var, const char *val /*, i static void SetEnv(struct passwd *pas) { #if SETENV_LEAKS - safe_setenv4(&env_var_user, "USER", pas->pw_name); - safe_setenv4(&env_var_home, "HOME", pas->pw_dir); + safe_setenv(&env_var_user, "USER", pas->pw_name); + safe_setenv(&env_var_home, "HOME", pas->pw_dir); /* if we want to set user's shell instead: */ - /*safe_setenv(env_var_user, "SHELL", pas->pw_shell, 5);*/ + /*safe_setenv(env_var_user, "SHELL", pas->pw_shell);*/ #else xsetenv("USER", pas->pw_name); xsetenv("HOME", pas->pw_dir); diff --git a/networking/tcpudp.c b/networking/tcpudp.c index 3b73f213f..55a3e0899 100644 --- a/networking/tcpudp.c +++ b/networking/tcpudp.c @@ -85,8 +85,7 @@ static void undo_xsetenv(void) char **pp = env_cur = &env_var[0]; while (*pp) { char *var = *pp; - *strchrnul(var, '=') = '\0'; - unsetenv(var); + bb_unsetenv(var); free(var); *pp++ = NULL; } diff --git a/networking/udhcp/script.c b/networking/udhcp/script.c index 4ae17fb8d..5d42a45db 100644 --- a/networking/udhcp/script.c +++ b/networking/udhcp/script.c @@ -119,7 +119,8 @@ static char *alloc_fill_opts(uint8_t *option, const struct dhcp_option *type_p, } option += optlen; len -= optlen; - if (len <= 0) break; + if (len <= 0) + break; dest += sprintf(dest, " "); } return ret; @@ -130,9 +131,8 @@ static char *alloc_fill_opts(uint8_t *option, const struct dhcp_option *type_p, static char **fill_envp(struct dhcpMessage *packet) { int num_options = 0; - int i, j; - char **envp; - char *var; + int i; + char **envp, **curr; const char *opt_name; uint8_t *temp; char over = 0; @@ -156,21 +156,16 @@ static char **fill_envp(struct dhcpMessage *packet) num_options++; } - envp = xzalloc(sizeof(char *) * (num_options + 5)); - j = 0; - envp[j++] = xasprintf("interface=%s", client_config.interface); - var = getenv("PATH"); - if (var) - envp[j++] = xasprintf("PATH=%s", var); - var = getenv("HOME"); - if (var) - envp[j++] = xasprintf("HOME=%s", var); + curr = envp = xzalloc(sizeof(char *) * (num_options + 3)); + *curr = xasprintf("interface=%s", client_config.interface); + putenv(*curr++); if (packet == NULL) return envp; - envp[j] = xmalloc(sizeof("ip=255.255.255.255")); - sprintip(envp[j++], "ip=", (uint8_t *) &packet->yiaddr); + *curr = xmalloc(sizeof("ip=255.255.255.255")); + sprintip(*curr, "ip=", (uint8_t *) &packet->yiaddr); + putenv(*curr++); opt_name = dhcp_option_strings; i = 0; @@ -178,31 +173,36 @@ static char **fill_envp(struct dhcpMessage *packet) temp = get_option(packet, dhcp_options[i].code); if (!temp) goto next; - envp[j++] = alloc_fill_opts(temp, &dhcp_options[i], opt_name); + *curr = alloc_fill_opts(temp, &dhcp_options[i], opt_name); + putenv(*curr++); /* Fill in a subnet bits option for things like /24 */ if (dhcp_options[i].code == DHCP_SUBNET) { uint32_t subnet; move_from_unaligned32(subnet, temp); - envp[j++] = xasprintf("mask=%d", mton(subnet)); + *curr = xasprintf("mask=%d", mton(subnet)); + putenv(*curr++); } next: opt_name += strlen(opt_name) + 1; i++; } if (packet->siaddr) { - envp[j] = xmalloc(sizeof("siaddr=255.255.255.255")); - sprintip(envp[j++], "siaddr=", (uint8_t *) &packet->siaddr); + *curr = xmalloc(sizeof("siaddr=255.255.255.255")); + sprintip(*curr, "siaddr=", (uint8_t *) &packet->siaddr); + putenv(*curr++); } if (!(over & FILE_FIELD) && packet->file[0]) { /* watch out for invalid packets */ packet->file[sizeof(packet->file) - 1] = '\0'; - envp[j++] = xasprintf("boot_file=%s", packet->file); + *curr = xasprintf("boot_file=%s", packet->file); + putenv(*curr++); } if (!(over & SNAME_FIELD) && packet->sname[0]) { /* watch out for invalid packets */ packet->sname[sizeof(packet->sname) - 1] = '\0'; - envp[j++] = xasprintf("sname=%s", packet->sname); + *curr = xasprintf("sname=%s", packet->sname); + putenv(*curr++); } return envp; } @@ -211,29 +211,25 @@ static char **fill_envp(struct dhcpMessage *packet) /* Call a script with a par file and env vars */ void FAST_FUNC udhcp_run_script(struct dhcpMessage *packet, const char *name) { - int pid; char **envp, **curr; + char *argv[3]; if (client_config.script == NULL) return; - DEBUG("vfork'ing and execle'ing %s", client_config.script); + DEBUG("vfork'ing and exec'ing %s", client_config.script); envp = fill_envp(packet); /* call script */ -// can we use wait4pid(spawn(...)) here? - pid = vfork(); - if (pid < 0) return; - if (pid == 0) { - /* close fd's? */ - /* exec script */ - execle(client_config.script, client_config.script, - name, NULL, envp); - bb_perror_msg_and_die("exec %s", client_config.script); - } - safe_waitpid(pid, NULL, 0); - for (curr = envp; *curr; curr++) + argv[0] = (char*) client_config.script; + argv[1] = (char*) name; + argv[2] = NULL; + wait4pid(spawn(argv)); + + for (curr = envp; *curr; curr++) { + bb_unsetenv(*curr); free(*curr); + } free(envp); } diff --git a/shell/hush.c b/shell/hush.c index eafcbb4c9..3b87855b6 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -730,16 +730,8 @@ static void free_strings_and_unsetenv(char **strings, int unset) v = strings; while (*v) { if (unset) { - char *copy; - /* *strchrnul(*v, '=') = '\0'; -- BAD - * In case *v was putenv'ed, we can't - * unsetenv(*v) after taking out '=': - * it won't work, env is modified by taking out! - * horror :( */ - copy = xstrndup(*v, strchrnul(*v, '=') - *v); - debug_printf_env("unsetenv '%s'\n", copy); - unsetenv(copy); - free(copy); + debug_printf_env("unsetenv '%s'\n", *v); + bb_unsetenv(*v); } free(*v++); } @@ -2937,7 +2929,7 @@ static void unset_local_var(const char *name) * is ro, and we cannot reach this code on the 1st pass */ prev->next = cur->next; debug_printf_env("%s: unsetenv '%s'\n", __func__, cur->varstr); - unsetenv(cur->varstr); + bb_unsetenv(cur->varstr); if (!cur->max_len) free(cur->varstr); free(cur); diff --git a/util-linux/mdev.c b/util-linux/mdev.c index 34cabc934..956de15ae 100644 --- a/util-linux/mdev.c +++ b/util-linux/mdev.c @@ -276,8 +276,7 @@ static void make_device(char *path, int delete) putenv(s); if (system(command) == -1) bb_perror_msg_and_die("can't run '%s'", command); - s[4] = '\0'; - unsetenv(s); + unsetenv("MDEV"); free(s); free(command); } -- 2.25.1