From 4d1a69043862ed979642f5688097160355d4cc81 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 11 Feb 2013 03:46:08 +0100 Subject: [PATCH] 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]) --- Makefile.am | 2 + src/core/dbus-manager.c | 1 + src/core/execute.c | 1 + src/core/service.c | 1 + src/hostname/hostnamed.c | 1 + src/locale/localed.c | 1 + src/notify/notify.c | 1 + src/shared/env-util.c | 394 ++++++++++++++++++++++++++++++++++++ src/shared/env-util.h | 41 ++++ src/shared/strv.c | 243 ---------------------- src/shared/strv.h | 11 - src/shared/util.c | 22 +- src/shared/util.h | 1 + src/test/test-env-replace.c | 44 +++- 14 files changed, 502 insertions(+), 262 deletions(-) create mode 100644 src/shared/env-util.c create mode 100644 src/shared/env-util.h diff --git a/Makefile.am b/Makefile.am index 8a7c2b67a..9da3394c7 100644 --- a/Makefile.am +++ b/Makefile.am @@ -573,6 +573,8 @@ libsystemd_shared_la_SOURCES = \ src/shared/fdset.h \ src/shared/strv.c \ src/shared/strv.h \ + src/shared/env-util.c \ + src/shared/env-util.h \ src/shared/strbuf.c \ src/shared/strbuf.h \ src/shared/strxcpyx.c \ diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index b7829572f..707119623 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -36,6 +36,7 @@ #include "path-util.h" #include "dbus-unit.h" #include "virt.h" +#include "env-util.h" #define BUS_MANAGER_INTERFACE_BEGIN \ " \n" diff --git a/src/core/execute.c b/src/core/execute.c index 1413c9110..aa58bc488 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -64,6 +64,7 @@ #include "loopback-setup.h" #include "path-util.h" #include "syscall-list.h" +#include "env-util.h" #define IDLE_TIMEOUT_USEC (5*USEC_PER_SEC) diff --git a/src/core/service.c b/src/core/service.c index 914146383..9c4bc4143 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -42,6 +42,7 @@ #include "path-util.h" #include "util.h" #include "utf8.h" +#include "env-util.h" #ifdef HAVE_SYSV_COMPAT diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c index 92b150bfc..c5a8b6faa 100644 --- a/src/hostname/hostnamed.c +++ b/src/hostname/hostnamed.c @@ -32,6 +32,7 @@ #include "polkit.h" #include "def.h" #include "virt.h" +#include "env-util.h" #define INTERFACE \ " \n" \ diff --git a/src/locale/localed.c b/src/locale/localed.c index bb2a3a2e5..6b1a793d3 100644 --- a/src/locale/localed.c +++ b/src/locale/localed.c @@ -31,6 +31,7 @@ #include "dbus-common.h" #include "polkit.h" #include "def.h" +#include "env-util.h" #define INTERFACE \ " \n" \ diff --git a/src/notify/notify.c b/src/notify/notify.c index f521f5665..1e9766f86 100644 --- a/src/notify/notify.c +++ b/src/notify/notify.c @@ -34,6 +34,7 @@ #include "log.h" #include "sd-readahead.h" #include "build.h" +#include "env-util.h" static bool arg_ready = false; static pid_t arg_pid = 0; diff --git a/src/shared/env-util.c b/src/shared/env-util.c new file mode 100644 index 000000000..7a213a77c --- /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 . +***/ + +#include +#include +#include + +#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 000000000..93bf596ca --- /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 . +***/ + +#include +#include + +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 fc6104ffe..ee0b71ece 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 33c752a31..d28625bd2 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 969ef2bb9..5b795d4a2 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 223617c3f..18494f14f 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 *), diff --git a/src/test/test-env-replace.c b/src/test/test-env-replace.c index 2da384535..b8747db68 100644 --- a/src/test/test-env-replace.c +++ b/src/test/test-env-replace.c @@ -24,6 +24,7 @@ #include "util.h" #include "strv.h" +#include "env-util.h" static void test_strv_env_delete(void) { _cleanup_strv_free_ char **a = NULL, **b = NULL, **c = NULL, **d = NULL; @@ -81,10 +82,12 @@ static void test_strv_env_merge(void) { assert(strv_length(r) == 6); strv_env_clean(r); - assert(streq(r[0], "PIEP")); - assert(streq(r[1], "SCHLUMPF=SMURFF")); - assert(streq(r[2], "NANANANA=YES")); - assert(strv_length(r) == 3); + assert(streq(r[0], "FOO=")); + assert(streq(r[1], "WALDO=")); + assert(streq(r[2], "SCHLUMPF=SMURFF")); + assert(streq(r[3], "PIEP=")); + assert(streq(r[4], "NANANANA=YES")); + assert(strv_length(r) == 5); } static void test_replace_env_arg(void) { @@ -145,6 +148,38 @@ static void test_normalize_env_assignment(void) { test_one_normalize(" ' xyz' = 'bar ' ", "' xyz'=bar "); } +static void test_env_clean(void) { + + _cleanup_strv_free_ char **e; + + e = strv_new("FOOBAR=WALDO", + "FOOBAR=WALDO", + "FOOBAR", + "F", + "X=", + "F=F", + "=", + "=F", + "", + "0000=000", + "äöüß=abcd", + "abcd=äöüß", + "xyz\n=xyz", + "xyz=xyz\n", + "another=one", + "another=final one", + NULL); + + assert_se(strv_env_clean(e)); + + assert_se(streq(e[0], "FOOBAR=WALDO")); + assert_se(streq(e[1], "X=")); + assert_se(streq(e[2], "F=F")); + assert_se(streq(e[3], "abcd=äöüß")); + assert_se(streq(e[4], "another=final one")); + assert_se(e[5] == NULL); +} + int main(int argc, char *argv[]) { test_strv_env_delete(); test_strv_env_unset(); @@ -152,6 +187,7 @@ int main(int argc, char *argv[]) { test_strv_env_merge(); test_replace_env_arg(); test_normalize_env_assignment(); + test_env_clean(); return 0; } -- 2.30.2