From: tedu Date: Thu, 16 Jun 2016 17:40:30 +0000 (+0000) Subject: the environment handling code was showing its age. just because environ is a char... X-Git-Tag: v0.3~7 X-Git-Url: https://git.armaanb.net/?p=opendoas.git;a=commitdiff_plain;h=1a0ed98a5cb619824028193ecff946f209da81fb the environment handling code was showing its age. just because environ is a char** array doesn't mean we must exclusively operate on such. convert to a red-black tree, manipulate as desired, then flatten to array. potentially overkill for the current operations, but reading the tea leaves i see that more manipulations are desired. ok tb (and some thought provoking disagreement from martijn) --- diff --git a/Makefile b/Makefile index 66792eb..fa69b8c 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ # $OpenBSD: Makefile,v 1.9 2014/01/13 01:41:00 tedu Exp $ -SRCS= parse.y doas.c +SRCS= parse.y doas.c env.c PROG= doas MAN= doas.1 doas.conf.5 diff --git a/doas.c b/doas.c index f1b2ec0..eef8955 100644 --- a/doas.c +++ b/doas.c @@ -190,105 +190,6 @@ parseconfig(const char *filename, int checkperms) exit(1); } -/* - * Copy the environment variables in safeset from oldenvp to envp. - */ -static int -copyenvhelper(const char **oldenvp, const char **safeset, size_t nsafe, - char **envp, int ei) -{ - size_t i; - - for (i = 0; i < nsafe; i++) { - const char **oe = oldenvp; - while (*oe) { - size_t len = strlen(safeset[i]); - if (strncmp(*oe, safeset[i], len) == 0 && - (*oe)[len] == '=') { - if (!(envp[ei++] = strdup(*oe))) - err(1, "strdup"); - break; - } - oe++; - } - } - return ei; -} - -static char ** -copyenv(const char **oldenvp, struct rule *rule) -{ - const char *safeset[] = { - "DISPLAY", "HOME", "LOGNAME", "MAIL", - "PATH", "TERM", "USER", "USERNAME", - NULL - }; - const char *badset[] = { - "ENV", - NULL - }; - char **envp; - const char **extra; - int ei; - size_t nsafe, nbad; - size_t nextras = 0; - - /* if there was no envvar whitelist, pass all except badset ones */ - nbad = arraylen(badset); - if ((rule->options & KEEPENV) && !rule->envlist) { - size_t iold, inew; - size_t oldlen = arraylen(oldenvp); - envp = reallocarray(NULL, oldlen + 1, sizeof(char *)); - if (!envp) - err(1, "reallocarray"); - for (inew = iold = 0; iold < oldlen; iold++) { - size_t ibad; - for (ibad = 0; ibad < nbad; ibad++) { - size_t len = strlen(badset[ibad]); - if (strncmp(oldenvp[iold], badset[ibad], len) == 0 && - oldenvp[iold][len] == '=') { - break; - } - } - if (ibad == nbad) { - if (!(envp[inew] = strdup(oldenvp[iold]))) - err(1, "strdup"); - inew++; - } - } - envp[inew] = NULL; - return envp; - } - - nsafe = arraylen(safeset); - if ((extra = rule->envlist)) { - size_t isafe; - nextras = arraylen(extra); - for (isafe = 0; isafe < nsafe; isafe++) { - size_t iextras; - for (iextras = 0; iextras < nextras; iextras++) { - if (strcmp(extra[iextras], safeset[isafe]) == 0) { - nextras--; - extra[iextras] = extra[nextras]; - extra[nextras] = NULL; - iextras--; - } - } - } - } - - envp = reallocarray(NULL, nsafe + nextras + 1, sizeof(char *)); - if (!envp) - err(1, "can't allocate new environment"); - - ei = 0; - ei = copyenvhelper(oldenvp, safeset, nsafe, envp, ei); - ei = copyenvhelper(oldenvp, rule->envlist, nextras, envp, ei); - envp[ei] = NULL; - - return envp; -} - static void __dead checkconfig(const char *confpath, int argc, char **argv, uid_t uid, gid_t *groups, int ngroups, uid_t target) @@ -321,6 +222,7 @@ main(int argc, char **argv, char **envp) char *shargv[] = { NULL, NULL }; char *sh; const char *cmd; + struct env *env; char cmdline[LINE_MAX]; char myname[_PW_NAME_LEN + 1]; struct passwd *pw; @@ -517,7 +419,9 @@ main(int argc, char **argv, char **envp) syslog(LOG_AUTHPRIV | LOG_INFO, "%s ran command %s as %s from %s", myname, cmdline, pw->pw_name, cwd); - envp = copyenv((const char **)envp, rule); + env = createenv(envp); + env = filterenv(env, rule); + envp = flattenenv(env); if (rule->cmd) { if (setenv("PATH", safepath, 1) == -1) diff --git a/doas.h b/doas.h index 235df9f..88b2223 100644 --- a/doas.h +++ b/doas.h @@ -1,5 +1,20 @@ /* $OpenBSD: doas.h,v 1.3 2015/07/21 11:04:06 zhuk Exp $ */ +#include + +struct envnode { + RB_ENTRY(envnode) node; + const char *key; + const char *value; +}; + +struct env { + RB_HEAD(envtree, envnode) root; + u_int count; +}; + +RB_PROTOTYPE(envtree, envnode, node, envcmp) + struct rule { int action; int options; @@ -16,6 +31,10 @@ extern int parse_errors; size_t arraylen(const char **); +struct env *createenv(char **); +struct env *filterenv(struct env *, struct rule *); +char **flattenenv(struct env *); + #define PERMIT 1 #define DENY 2 diff --git a/env.c b/env.c new file mode 100644 index 0000000..cf51e67 --- /dev/null +++ b/env.c @@ -0,0 +1,153 @@ +/* $OpenBSD$ */ +/* + * Copyright (c) 2016 Ted Unangst + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#include + +#include +#include +#include +#include +#include +#include + +#include "doas.h" + +int +envcmp(struct envnode *a, struct envnode *b) +{ + return strcmp(a->key, b->key); +} +RB_GENERATE(envtree, envnode, node, envcmp) + +struct env * +createenv(char **envp) +{ + struct env *env; + u_int i; + + env = malloc(sizeof(*env)); + if (!env) + err(1, NULL); + RB_INIT(&env->root); + env->count = 0; + + for (i = 0; envp[i] != NULL; i++) { + struct envnode *node; + const char *e, *eq; + + e = envp[i]; + + if ((eq = strchr(e, '=')) == NULL || eq == e) + continue; + node = malloc(sizeof(*node)); + if (!node) + err(1, NULL); + node->key = strndup(envp[i], eq - e); + node->value = strdup(eq + 1); + if (!node->key || !node->value) + err(1, NULL); + if (RB_FIND(envtree, &env->root, node)) { + free((char *)node->key); + free((char *)node->value); + free(node); + } else { + RB_INSERT(envtree, &env->root, node); + env->count++; + } + } + return env; +} + +char ** +flattenenv(struct env *env) +{ + char **envp; + struct envnode *node; + u_int i; + + envp = reallocarray(NULL, env->count + 1, sizeof(char *)); + if (!envp) + err(1, NULL); + i = 0; + RB_FOREACH(node, envtree, &env->root) { + if (asprintf(&envp[i], "%s=%s", node->key, node->value) == -1) + err(1, NULL); + i++; + } + envp[i] = NULL; + return envp; +} + +static void +copyenv(struct env *orig, struct env *copy, const char **envlist) +{ + struct envnode *node, key; + u_int i; + + for (i = 0; envlist[i]; i++) { + key.key = envlist[i]; + if ((node = RB_FIND(envtree, &orig->root, &key))) { + RB_REMOVE(envtree, &orig->root, node); + orig->count--; + RB_INSERT(envtree, ©->root, node); + copy->count++; + } + } +} + +struct env * +filterenv(struct env *orig, struct rule *rule) +{ + const char *safeset[] = { + "DISPLAY", "HOME", "LOGNAME", "MAIL", + "PATH", "TERM", "USER", "USERNAME", + NULL + }; + const char *badset[] = { + "ENV", + NULL + }; + struct env *copy; + struct envnode *node, key; + u_int i; + + if ((rule->options & KEEPENV) && !rule->envlist) { + for (i = 0; badset[i]; i++) { + key.key = badset[i]; + if ((node = RB_FIND(envtree, &orig->root, &key))) { + RB_REMOVE(envtree, &orig->root, node); + free((char *)node->key); + free((char *)node->value); + free(node); + orig->count--; + } + } + return orig; + } + + copy = malloc(sizeof(*copy)); + if (!copy) + err(1, NULL); + RB_INIT(©->root); + copy->count = 0; + + if (rule->envlist) + copyenv(orig, copy, rule->envlist); + copyenv(orig, copy, safeset); + + return copy; +}