env: considerably beef up environment cleaning logic

Now, actually check if the environment variable names and values used
are valid, before accepting them. With this in place are at some places
more rigid than POSIX, and less rigid at others. For example, this code
allows lower-case environment variables (which POSIX suggests not to
use), but it will not allow non-UTF8 variable values.

All in all this should be a good middle ground of what to allow and what
not to allow as environment variables.

(This also splits out all environment related calls into env-util.[ch])

6 files changed, 454 insertions, 258 deletions

diff --git a/src/shared/env-util.c b/src/shared/env-util.c
new file mode 100644
index 0000000000..7a213a77c0
--- /dev/null
+++ b/src/shared/env-util.c
@@ -0,0 +1,394 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2012 Lennart Poettering
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include <limits.h>
+#include <sys/param.h>
+#include <unistd.h>
+
+#include "strv.h"
+#include "utf8.h"
+#include "util.h"
+#include "env-util.h"
+
+#define VALID_CHARS_ENV_NAME                    \
+        "0123456789"                            \
+        "abcdefghijklmnopqrstuvwxyz"            \
+        "ABCDEFGHIJKLMNOPQRSTUVWXYZ"            \
+        "_"
+
+#ifndef ARG_MAX
+#define ARG_MAX ((size_t) sysconf(_SC_ARG_MAX))
+#endif
+
+static bool env_name_is_valid_n(const char *e, size_t n) {
+        const char *p;
+
+        if (!e)
+                return false;
+
+        if (n <= 0)
+                return false;
+
+        if (e[0] >= '0' && e[0] <= '9')
+                return false;
+
+        /* POSIX says the overall size of the environment block cannot
+         * be > ARG_MAX, an individual assignment hence cannot be
+         * either. Discounting the equal sign and trailing NUL this
+         * hence leaves ARG_MAX-2 as longest possible variable
+         * name. */
+        if (n > ARG_MAX - 2)
+                return false;
+
+        for (p = e; p < e + n; p++)
+                if (!strchr(VALID_CHARS_ENV_NAME, *p))
+                        return false;
+
+        return true;
+}
+
+bool env_name_is_valid(const char *e) {
+        if (!e)
+                return false;
+
+        return env_name_is_valid_n(e, strlen(e));
+}
+
+bool env_value_is_valid(const char *e) {
+        if (!e)
+                return false;
+
+        if (!utf8_is_valid(e))
+                return false;
+
+        if (string_has_cc(e))
+                return false;
+
+        /* POSIX says the overall size of the environment block cannot
+         * be > ARG_MAX, an individual assignment hence cannot be
+         * either. Discounting the shortest possible variable name of
+         * length 1, the equal sign and trailing NUL this hence leaves
+         * ARG_MAX-3 as longest possible variable value. */
+        if (strlen(e) > ARG_MAX - 3)
+                return false;
+
+        return true;
+}
+
+bool env_assignment_is_valid(const char *e) {
+        const char *eq;
+
+        eq = strchr(e, '=');
+        if (!eq)
+                return false;
+
+        if (!env_name_is_valid_n(e, eq - e))
+                return false;
+
+        if (!env_value_is_valid(eq + 1))
+                return false;
+
+        /* POSIX says the overall size of the environment block cannot
+         * be > ARG_MAX, hence the individual variable assignments
+         * cannot be either, but let's room for one trailing NUL
+         * byte. */
+        if (strlen(e) > ARG_MAX - 1)
+                return false;
+
+        return true;
+}
+
+bool strv_env_is_valid(char **e) {
+        char **p, **q;
+
+        STRV_FOREACH(p, e) {
+                size_t k;
+
+                if (!env_assignment_is_valid(*p))
+                        return false;
+
+                /* Check if there are duplicate assginments */
+                k = strcspn(*p, "=");
+                STRV_FOREACH(q, p + 1)
+                        if (strncmp(*p, *q, k) == 0 && (*q)[k] == '=')
+                                return false;
+        }
+
+        return true;
+}
+
+static int env_append(char **r, char ***k, char **a) {
+        assert(r);
+        assert(k);
+
+        if (!a)
+                return 0;
+
+        /* Add the entries of a to *k unless they already exist in *r
+         * in which case they are overridden instead. This assumes
+         * there is enough space in the r array. */
+
+        for (; *a; a++) {
+                char **j;
+                size_t n;
+
+                n = strcspn(*a, "=");
+
+                if ((*a)[n] == '=')
+                        n++;
+
+                for (j = r; j < *k; j++)
+                        if (strncmp(*j, *a, n) == 0)
+                                break;
+
+                if (j >= *k)
+                        (*k)++;
+                else
+                        free(*j);
+
+                *j = strdup(*a);
+                if (!*j)
+                        return -ENOMEM;
+        }
+
+        return 0;
+}
+
+char **strv_env_merge(unsigned n_lists, ...) {
+        size_t n = 0;
+        char **l, **k, **r;
+        va_list ap;
+        unsigned i;
+
+        /* Merges an arbitrary number of environment sets */
+
+        va_start(ap, n_lists);
+        for (i = 0; i < n_lists; i++) {
+                l = va_arg(ap, char**);
+                n += strv_length(l);
+        }
+        va_end(ap);
+
+        r = new(char*, n+1);
+        if (!r)
+                return NULL;
+
+        k = r;
+
+        va_start(ap, n_lists);
+        for (i = 0; i < n_lists; i++) {
+                l = va_arg(ap, char**);
+                if (env_append(r, &k, l) < 0)
+                        goto fail;
+        }
+        va_end(ap);
+
+        *k = NULL;
+
+        return r;
+
+fail:
+        va_end(ap);
+        strv_free(r);
+
+        return NULL;
+}
+
+static bool env_match(const char *t, const char *pattern) {
+        assert(t);
+        assert(pattern);
+
+        /* pattern a matches string a
+         *         a matches a=
+         *         a matches a=b
+         *         a= matches a=
+         *         a=b matches a=b
+         *         a= does not match a
+         *         a=b does not match a=
+         *         a=b does not match a
+         *         a=b does not match a=c */
+
+        if (streq(t, pattern))
+                return true;
+
+        if (!strchr(pattern, '=')) {
+                size_t l = strlen(pattern);
+
+                return strncmp(t, pattern, l) == 0 && t[l] == '=';
+        }
+
+        return false;
+}
+
+char **strv_env_delete(char **x, unsigned n_lists, ...) {
+        size_t n, i = 0;
+        char **k, **r;
+        va_list ap;
+
+        /* Deletes every entry from x that is mentioned in the other
+         * string lists */
+
+        n = strv_length(x);
+
+        r = new(char*, n+1);
+        if (!r)
+                return NULL;
+
+        STRV_FOREACH(k, x) {
+                unsigned v;
+
+                va_start(ap, n_lists);
+                for (v = 0; v < n_lists; v++) {
+                        char **l, **j;
+
+                        l = va_arg(ap, char**);
+                        STRV_FOREACH(j, l)
+                                if (env_match(*k, *j))
+                                        goto skip;
+                }
+                va_end(ap);
+
+                r[i] = strdup(*k);
+                if (!r[i]) {
+                        strv_free(r);
+                        return NULL;
+                }
+
+                i++;
+                continue;
+
+        skip:
+                va_end(ap);
+        }
+
+        r[i] = NULL;
+
+        assert(i <= n);
+
+        return r;
+}
+
+char **strv_env_unset(char **l, const char *p) {
+
+        char **f, **t;
+
+        if (!l)
+                return NULL;
+
+        assert(p);
+
+        /* Drops every occurrence of the env var setting p in the
+         * string list. edits in-place. */
+
+        for (f = t = l; *f; f++) {
+
+                if (env_match(*f, p)) {
+                        free(*f);
+                        continue;
+                }
+
+                *(t++) = *f;
+        }
+
+        *t = NULL;
+        return l;
+}
+
+char **strv_env_set(char **x, const char *p) {
+
+        char **k, **r;
+        char* m[2] = { (char*) p, NULL };
+
+        /* Overrides the env var setting of p, returns a new copy */
+
+        r = new(char*, strv_length(x)+2);
+        if (!r)
+                return NULL;
+
+        k = r;
+        if (env_append(r, &k, x) < 0)
+                goto fail;
+
+        if (env_append(r, &k, m) < 0)
+                goto fail;
+
+        *k = NULL;
+
+        return r;
+
+fail:
+        strv_free(r);
+        return NULL;
+}
+
+char *strv_env_get_n(char **l, const char *name, size_t k) {
+        char **i;
+
+        assert(name);
+
+        if (k <= 0)
+                return NULL;
+
+        STRV_FOREACH(i, l)
+                if (strncmp(*i, name, k) == 0 &&
+                    (*i)[k] == '=')
+                        return *i + k + 1;
+
+        return NULL;
+}
+
+char *strv_env_get(char **l, const char *name) {
+        assert(name);
+
+        return strv_env_get_n(l, name, strlen(name));
+}
+
+char **strv_env_clean(char **e) {
+        char **p, **q;
+        int k = 0;
+
+        STRV_FOREACH(p, e) {
+                size_t n;
+                bool duplicate = false;
+
+                if (!env_assignment_is_valid(*p)) {
+                        free(*p);
+                        continue;
+                }
+
+                n = strcspn(*p, "=");
+                STRV_FOREACH(q, p + 1)
+                        if (strncmp(*p, *q, n) == 0 && (*q)[n] == '=') {
+                                duplicate = true;
+                                break;
+                        }
+
+                if (duplicate) {
+                        free(*p);
+                        continue;
+                }
+
+                e[k++] = *p;
+        }
+
+        e[k] = NULL;
+        return e;
+}
diff --git a/src/shared/env-util.h b/src/shared/env-util.h
new file mode 100644
index 0000000000..93bf596ca8
--- /dev/null
+++ b/src/shared/env-util.h
@@ -0,0 +1,41 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+#pragma once
+
+/***
+  This file is part of systemd.
+
+  Copyright 2013 Lennart Poettering
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include <stdbool.h>
+#include <sys/types.h>
+
+bool env_name_is_valid(const char *e);
+bool env_value_is_valid(const char *e);
+bool env_assignment_is_valid(const char *e);
+
+bool strv_env_is_valid(char **e);
+char **strv_env_clean(char **l);
+
+char **strv_env_merge(unsigned n_lists, ...);
+char **strv_env_delete(char **x, unsigned n_lists, ...); /* New copy */
+
+char **strv_env_set(char **x, const char *p); /* New copy ... */
+char **strv_env_unset(char **l, const char *p); /* In place ... */
+
+char *strv_env_get_n(char **l, const char *name, size_t k);
+char *strv_env_get(char **x, const char *n);
diff --git a/src/shared/strv.c b/src/shared/strv.c
index fc6104ffea..ee0b71ece0 100644
--- a/src/shared/strv.c
+++ b/src/shared/strv.c
@@ -458,249 +458,6 @@ char **strv_remove_prefix(char **l, const char *s) {
         return l;
 }
 
-static int env_append(char **r, char ***k, char **a) {
-        assert(r);
-        assert(k);
-
-        if (!a)
-                return 0;
-
-        /* Add the entries of a to *k unless they already exist in *r
-         * in which case they are overridden instead. This assumes
-         * there is enough space in the r array. */
-
-        for (; *a; a++) {
-                char **j;
-                size_t n;
-
-                n = strcspn(*a, "=");
-
-                if ((*a)[n] == '=')
-                        n++;
-
-                for (j = r; j < *k; j++)
-                        if (strncmp(*j, *a, n) == 0)
-                                break;
-
-                if (j >= *k)
-                        (*k)++;
-                else
-                        free(*j);
-
-                *j = strdup(*a);
-                if (!*j)
-                        return -ENOMEM;
-        }
-
-        return 0;
-}
-
-char **strv_env_merge(unsigned n_lists, ...) {
-        size_t n = 0;
-        char **l, **k, **r;
-        va_list ap;
-        unsigned i;
-
-        /* Merges an arbitrary number of environment sets */
-
-        va_start(ap, n_lists);
-        for (i = 0; i < n_lists; i++) {
-                l = va_arg(ap, char**);
-                n += strv_length(l);
-        }
-        va_end(ap);
-
-        r = new(char*, n+1);
-        if (!r)
-                return NULL;
-
-        k = r;
-
-        va_start(ap, n_lists);
-        for (i = 0; i < n_lists; i++) {
-                l = va_arg(ap, char**);
-                if (env_append(r, &k, l) < 0)
-                        goto fail;
-        }
-        va_end(ap);
-
-        *k = NULL;
-
-        return r;
-
-fail:
-        va_end(ap);
-        strv_free(r);
-
-        return NULL;
-}
-
-static bool env_match(const char *t, const char *pattern) {
-        assert(t);
-        assert(pattern);
-
-        /* pattern a matches string a
-         *         a matches a=
-         *         a matches a=b
-         *         a= matches a=
-         *         a=b matches a=b
-         *         a= does not match a
-         *         a=b does not match a=
-         *         a=b does not match a
-         *         a=b does not match a=c */
-
-        if (streq(t, pattern))
-                return true;
-
-        if (!strchr(pattern, '=')) {
-                size_t l = strlen(pattern);
-
-                return strncmp(t, pattern, l) == 0 && t[l] == '=';
-        }
-
-        return false;
-}
-
-char **strv_env_delete(char **x, unsigned n_lists, ...) {
-        size_t n, i = 0;
-        char **k, **r;
-        va_list ap;
-
-        /* Deletes every entry from x that is mentioned in the other
-         * string lists */
-
-        n = strv_length(x);
-
-        r = new(char*, n+1);
-        if (!r)
-                return NULL;
-
-        STRV_FOREACH(k, x) {
-                unsigned v;
-
-                va_start(ap, n_lists);
-                for (v = 0; v < n_lists; v++) {
-                        char **l, **j;
-
-                        l = va_arg(ap, char**);
-                        STRV_FOREACH(j, l)
-                                if (env_match(*k, *j))
-                                        goto skip;
-                }
-                va_end(ap);
-
-                r[i] = strdup(*k);
-                if (!r[i]) {
-                        strv_free(r);
-                        return NULL;
-                }
-
-                i++;
-                continue;
-
-        skip:
-                va_end(ap);
-        }
-
-        r[i] = NULL;
-
-        assert(i <= n);
-
-        return r;
-}
-
-char **strv_env_unset(char **l, const char *p) {
-
-        char **f, **t;
-
-        if (!l)
-                return NULL;
-
-        assert(p);
-
-        /* Drops every occurrence of the env var setting p in the
-         * string list. edits in-place. */
-
-        for (f = t = l; *f; f++) {
-
-                if (env_match(*f, p)) {
-                        free(*f);
-                        continue;
-                }
-
-                *(t++) = *f;
-        }
-
-        *t = NULL;
-        return l;
-}
-
-char **strv_env_set(char **x, const char *p) {
-
-        char **k, **r;
-        char* m[2] = { (char*) p, NULL };
-
-        /* Overrides the env var setting of p, returns a new copy */
-
-        r = new(char*, strv_length(x)+2);
-        if (!r)
-                return NULL;
-
-        k = r;
-        if (env_append(r, &k, x) < 0)
-                goto fail;
-
-        if (env_append(r, &k, m) < 0)
-                goto fail;
-
-        *k = NULL;
-
-        return r;
-
-fail:
-        strv_free(r);
-        return NULL;
-
-}
-
-char *strv_env_get_with_length(char **l, const char *name, size_t k) {
-        char **i;
-
-        assert(name);
-
-        STRV_FOREACH(i, l)
-                if (strncmp(*i, name, k) == 0 &&
-                    (*i)[k] == '=')
-                        return *i + k + 1;
-
-        return NULL;
-}
-
-char *strv_env_get(char **l, const char *name) {
-        return strv_env_get_with_length(l, name, strlen(name));
-}
-
-char **strv_env_clean(char **l) {
-        char **r, **ret;
-
-        for (r = ret = l; *l; l++) {
-                const char *equal;
-
-                equal = strchr(*l, '=');
-
-                if (equal && equal[1] == 0) {
-                        free(*l);
-                        continue;
-                }
-
-                *(r++) = *l;
-        }
-
-        *r = NULL;
-
-        return ret;
-}
-
 char **strv_parse_nulstr(const char *s, size_t l) {
         const char *p;
         unsigned c = 0, i = 0;
diff --git a/src/shared/strv.h b/src/shared/strv.h
index 33c752a313..d28625bd2f 100644
--- a/src/shared/strv.h
+++ b/src/shared/strv.h
@@ -61,17 +61,6 @@ char **strv_split_quoted(const char *s) _malloc_;
 
 char *strv_join(char **l, const char *separator) _malloc_;
 
-char **strv_env_merge(unsigned n_lists, ...);
-char **strv_env_delete(char **x, unsigned n_lists, ...);
-
-char **strv_env_set(char **x, const char *p);
-char **strv_env_unset(char **l, const char *p);
-
-char *strv_env_get_with_length(char **l, const char *name, size_t k);
-char *strv_env_get(char **x, const char *n);
-
-char **strv_env_clean(char **l);
-
 char **strv_parse_nulstr(const char *s, size_t l);
 
 bool strv_overlap(char **a, char **b);
diff --git a/src/shared/util.c b/src/shared/util.c
index 969ef2bb90..5b795d4a24 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -70,6 +70,7 @@
 #include "path-util.h"
 #include "exit-status.h"
 #include "hashmap.h"
+#include "env-util.h"
 
 int saved_argc = 0;
 char **saved_argv = NULL;
@@ -3341,10 +3342,10 @@ char *replace_env(const char *format, char **env) {
                         if (*e == '}') {
                                 const char *t;
 
-                                if (!(t = strv_env_get_with_length(env, word+2, e-word-2)))
-                                        t = "";
+                                t = strempty(strv_env_get_n(env, word+2, e-word-2));
 
-                                if (!(k = strappend(r, t)))
+                                k = strappend(r, t);
+                                if (!k)
                                         goto fail;
 
                                 free(r);
@@ -3385,7 +3386,8 @@ char **replace_env_argv(char **argv, char **env) {
                         char **w, **m;
                         unsigned q;
 
-                        if ((e = strv_env_get(env, *i+1))) {
+                        e = strv_env_get(env, *i+1);
+                        if (e) {
 
                                 if (!(m = strv_split_quoted(e))) {
                                         r[k] = NULL;
@@ -5608,6 +5610,18 @@ bool string_is_safe(const char *p) {
         return true;
 }
 
+bool string_has_cc(const char *p) {
+        const char *t;
+
+        assert(p);
+
+        for (t = p; *t; t++)
+                if (*t > 0 && *t < ' ')
+                        return true;
+
+        return false;
+}
+
 bool path_is_safe(const char *p) {
 
         if (isempty(p))
diff --git a/src/shared/util.h b/src/shared/util.h
index 223617c3ff..18494f14f2 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -544,6 +544,7 @@ _malloc_ static inline void *memdup_multiply(const void *p, size_t a, size_t b)
 bool filename_is_safe(const char *p);
 bool path_is_safe(const char *p);
 bool string_is_safe(const char *p);
+bool string_has_cc(const char *p);
 
 void *xbsearch_r(const void *key, const void *base, size_t nmemb, size_t size,
                  int (*compar) (const void *, const void *, void *),