Instead of having to use another 1KB buffer just to figure out where
the '=' sign is, use the parser infrastructure already present. Also
allows for variable timeout option. Another (negligible) advantage: if
one day someone wants to change the maximum line length, they only need
to do so in one spot.

I haven't touched the code in env.c that handles the '-' and '$' since
these are extremely simple anyway, and aren't needed for the next patch.

This patch doesn't need changes to doas(1) or doas.conf(5) man pages, but there
is no mention of characters that are required to be escaped when specifying 
args.
Tested with all kinds of variable names and values, works fine:

        ... keepenv { FIRE SWITCH=1 NO_PLACE_LIKE=$HOME WATER}

Things I would like feedback on, in particular:

1. Loops - Use C99 style initialisation?
        for(int i = 0; i < monsters; i < 666)
                ...

2. Initialisation of arrays of two-member structs, where one member
   is initially set to NULL:

        struct envar safeset[] = {{"DISPLAY"}, {"HOME"}, ...};
   or
        struct envar safeset[] = {{"DISPLAY",NULL}, {"HOME",NULL}, ...};

3. Anything of major concern.

 - - - - - - - - - - - - - - - - - 

Index: usr.bin/doas/doas.h
===================================================================
RCS file: /cvs/src/usr.bin/doas/doas.h,v
retrieving revision 1.12
diff -u -p -r1.12 doas.h
--- usr.bin/doas/doas.h 5 Oct 2016 17:40:25 -0000       1.12
+++ usr.bin/doas/doas.h 15 Mar 2017 15:11:31 -0000
@@ -15,6 +15,11 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+struct envar {
+       const char *name;
+       const char *value;
+};
+
 struct rule {
        int action;
        int options;
@@ -22,7 +27,7 @@ struct rule {
        const char *target;
        const char *cmd;
        const char **cmdargs;
-       const char **envlist;
+       struct envar *envlist;
 };
 
 extern struct rule **rules;
Index: usr.bin/doas/env.c
===================================================================
RCS file: /cvs/src/usr.bin/doas/env.c,v
retrieving revision 1.5
diff -u -p -r1.5 env.c
--- usr.bin/doas/env.c  15 Sep 2016 00:58:23 -0000      1.5
+++ usr.bin/doas/env.c  15 Mar 2017 15:11:31 -0000
@@ -134,51 +134,39 @@ flattenenv(struct env *env)
 }
 
 static void
-fillenv(struct env *env, const char **envlist)
+fillenv(struct env *env, const struct envar *envlist)
 {
        struct envnode *node, key;
-       const char *e, *eq;
+       struct envar ev;
        const char *val;
-       char name[1024];
        u_int i;
-       size_t len;
 
-       for (i = 0; envlist[i]; i++) {
-               e = envlist[i];
-
-               /* parse out env name */
-               if ((eq = strchr(e, '=')) == NULL)
-                       len = strlen(e);
-               else
-                       len = eq - e;
-               if (len > sizeof(name) - 1)
-                       continue;
-               memcpy(name, e, len);
-               name[len] = '\0';
+       for (i = 0; envlist[i].name; i++) {
+               ev = envlist[i];
 
                /* delete previous copies */
-               key.key = name;
-               if (*name == '-')
-                       key.key = name + 1;
+               key.key = ev.name;
+               if (*ev.name == '-')
+                       key.key = ev.name + 1;
                if ((node = RB_FIND(envtree, &env->root, &key))) {
                        RB_REMOVE(envtree, &env->root, node);
                        freenode(node);
                        env->count--;
                }
-               if (*name == '-')
+               if (*ev.name == '-')
                        continue;
 
                /* assign value or inherit from environ */
-               if (eq) {
-                       val = eq + 1;
+               if (ev.value) {
+                       val = ev.value;
                        if (*val == '$')
                                val = getenv(val + 1);
                } else {
-                       val = getenv(name);
+                       val = getenv(ev.name);
                }
                /* at last, we have something to insert */
                if (val) {
-                       node = createnode(name, val);
+                       node = createnode(ev.name, val);
                        RB_INSERT(envtree, &env->root, node);
                        env->count++;
                }
@@ -188,10 +176,10 @@ fillenv(struct env *env, const char **en
 char **
 prepenv(struct rule *rule)
 {
-       static const char *safeset[] = {
-               "DISPLAY", "HOME", "LOGNAME", "MAIL",
-               "PATH", "TERM", "USER", "USERNAME",
-               NULL
+       static const struct envar safeset[] = {
+               {"DISPLAY"}, {"HOME"}, {"LOGNAME"}, {"MAIL"},
+               {"PATH"}, {"TERM"}, {"USER"}, {"USERNAME"},
+               {NULL}
        };
        struct env *env;
 
Index: usr.bin/doas/parse.y
===================================================================
RCS file: /cvs/src/usr.bin/doas/parse.y,v
retrieving revision 1.26
diff -u -p -r1.26 parse.y
--- usr.bin/doas/parse.y        2 Jan 2017 01:40:20 -0000       1.26
+++ usr.bin/doas/parse.y        15 Mar 2017 15:11:32 -0000
@@ -34,7 +34,7 @@ typedef struct {
                        int options;
                        const char *cmd;
                        const char **cmdargs;
-                       const char **envlist;
+                       struct envar *envlist;
                };
                const char **strlist;
                const char *str;
@@ -67,6 +67,18 @@ arraylen(const char **arr)
        return cnt;
 }
 
+static size_t
+envlistlen(const struct envar *el)
+{
+       size_t cnt = 0;
+
+       while (el->name) {
+               cnt++;
+               el++;
+       }
+       return cnt;
+}
+
 %}
 
 %token TPERMIT TDENY TAS TCMD TARGS
@@ -142,9 +154,9 @@ option:             TNOPASS {
                } | TKEEPENV {
                        $$.options = KEEPENV;
                        $$.envlist = NULL;
-               } | TSETENV '{' strlist '}' {
+               } | TSETENV '{' envlist '}' {
                        $$.options = 0;
-                       $$.envlist = $3.strlist;
+                       $$.envlist = $3.envlist;
                } ;
 
 strlist:       /* empty */ {
@@ -159,6 +171,27 @@ strlist:   /* empty */ {
                        $$.strlist[nstr + 1] = NULL;
                } ;
 
+envlist:       /* empty */ {
+                       if (!($$.envlist = calloc(1, sizeof(struct envar))))
+                               errx(1, "can't allocate envlist");
+               } | envlist TSTRING {
+                       int nenv = envlistlen($1.envlist);
+                       if (!($$.envlist = reallocarray($1.envlist, nenv + 2,
+                           sizeof(struct envar))))
+                               errx(1, "can't allocate envlist");
+                       $$.envlist[nenv].name = $2.str;
+                       $$.envlist[nenv].value = NULL;
+                       nenv++;
+                       $$.envlist[nenv].name = $$.envlist[nenv].value = NULL;
+               } | envlist '=' TSTRING {
+                       /* append a value to the last variable */
+                       int nenv = envlistlen($1.envlist);
+                       if(nenv < 1) {
+                               yyerror("expected variable name before '='");
+                               YYERROR;
+                       }
+                       $1.envlist[nenv - 1].value = $3.str;
+               } ;
 
 ident:         TSTRING {
                        $$.str = $1.str;
@@ -234,6 +267,7 @@ repeat:
                        yylval.colno = 0;
                        yylval.lineno++;
                        /* FALLTHROUGH */
+               case '=':
                case '{':
                case '}':
                        return c;
@@ -283,6 +317,7 @@ repeat:
                                    qpos + 1);
                        goto eow;
                        /* FALLTHROUGH */
+               case '=':
                case '{':
                case '}':
                case '#':

Reply via email to