chiark / gitweb /
env: considerably beef up environment cleaning logic
authorLennart Poettering <lennart@poettering.net>
Mon, 11 Feb 2013 02:46:08 +0000 (03:46 +0100)
committerLennart Poettering <lennart@poettering.net>
Mon, 11 Feb 2013 02:54:50 +0000 (03:54 +0100)
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])

14 files changed:
Makefile.am
src/core/dbus-manager.c
src/core/execute.c
src/core/service.c
src/hostname/hostnamed.c
src/locale/localed.c
src/notify/notify.c
src/shared/env-util.c [new file with mode: 0644]
src/shared/env-util.h [new file with mode: 0644]
src/shared/strv.c
src/shared/strv.h
src/shared/util.c
src/shared/util.h
src/test/test-env-replace.c

index 8a7c2b67aaf5d9677b4f6f7b7a55d8e1f0ea25da..9da3394c78308118e2bbbee9b91fe15ee6e1ee25 100644 (file)
@@ -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 \
index b7829572f55cab7b409d83e6d066146a17b8c31b..70711962388971da34addd08f73ec400bb84a606 100644 (file)
@@ -36,6 +36,7 @@
 #include "path-util.h"
 #include "dbus-unit.h"
 #include "virt.h"
+#include "env-util.h"
 
 #define BUS_MANAGER_INTERFACE_BEGIN                                     \
         " <interface name=\"org.freedesktop.systemd1.Manager\">\n"
index 1413c9110ee71dccdebb6eddc4d477e215755981..aa58bc488ca3a13f02b9badccc01a9639bbcb626 100644 (file)
@@ -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)
 
index 914146383ca50c1b21ed2ce4b8d16288fbbbc080..9c4bc414307f5baf12ff23ee2947a4720713a059 100644 (file)
@@ -42,6 +42,7 @@
 #include "path-util.h"
 #include "util.h"
 #include "utf8.h"
+#include "env-util.h"
 
 #ifdef HAVE_SYSV_COMPAT
 
index 92b150bfc9d5e785770b853bb8efaa3b3885a7c9..c5a8b6faafc1b45bd2a8ba283573066f306d8a93 100644 (file)
@@ -32,6 +32,7 @@
 #include "polkit.h"
 #include "def.h"
 #include "virt.h"
+#include "env-util.h"
 
 #define INTERFACE \
         " <interface name=\"org.freedesktop.hostname1\">\n"             \
index bb2a3a2e5422cea56ac865052f836c80b1de11c4..6b1a793d3b44f2aab179f49978985ca8a755aa6b 100644 (file)
@@ -31,6 +31,7 @@
 #include "dbus-common.h"
 #include "polkit.h"
 #include "def.h"
+#include "env-util.h"
 
 #define INTERFACE                                                       \
         " <interface name=\"org.freedesktop.locale1\">\n"               \
index f521f56659b7f5cc2698e60f058ed94819abcf8a..1e9766f86213fce91f3ae12e1ddeb6c676d68d26 100644 (file)
@@ -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 (file)
index 0000000..7a213a7
--- /dev/null
@@ -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 (file)
index 0000000..93bf596
--- /dev/null
@@ -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);
index fc6104ffeaaba438065c3a9e594f923a429e21b1..ee0b71ece048bf3a4aa59823ccd654912d38038b 100644 (file)
@@ -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;
index 33c752a313c0b36990c5e4da7825c49ec74aedff..d28625bd2f6ec9747fb843c151d538d2ef38ba1a 100644 (file)
@@ -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);
index 969ef2bb90a759409178ff109c29bc2c11a740fa..5b795d4a246e1e2be581479cb46acdd848b35647 100644 (file)
@@ -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))
index 223617c3ff6284f17fc329ce1134cce98c402ff8..18494f14f2ff31a6ead6507733fd10a1996a22fc 100644 (file)
@@ -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 *),
index 2da3845354ae0409f8231198ea0e7dea191bd1dd..b8747db6810a5b64857c756467d1185f40dec080 100644 (file)
@@ -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;
 }