From 6b9f600772c7f9b86c437c72d9a8e9ae6e8d9596 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 26 Jun 2023 17:15:28 +0200 Subject: [PATCH 1/6] string-util: add strextendn() helper --- src/basic/string-util.c | 27 +++++++++++++++++++++++++++ src/basic/string-util.h | 2 ++ src/test/test-string-util.c | 18 ++++++++++++++++++ 3 files changed, 47 insertions(+) diff --git a/src/basic/string-util.c b/src/basic/string-util.c index 1eedcb66f7..d5e1d4e92d 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -891,6 +891,33 @@ oom: return -ENOMEM; } +char *strextendn(char **x, const char *s, size_t l) { + assert(x); + assert(s || l == 0); + + if (l == SIZE_MAX) + l = strlen_ptr(s); + else if (l > 0) + l = strnlen(s, l); /* ignore trailing noise */ + + if (l > 0 || !*x) { + size_t q; + char *m; + + q = strlen_ptr(*x); + m = realloc(*x, q + l + 1); + if (!m) + return NULL; + + memcpy_safe(m + q, s, l); + m[q + l] = 0; + + *x = m; + } + + return *x; +} + char *strrep(const char *s, unsigned n) { char *r, *p; size_t l; diff --git a/src/basic/string-util.h b/src/basic/string-util.h index 0ff5b46bba..ce6ea64bb9 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -186,6 +186,8 @@ char *strextend_with_separator_internal(char **x, const char *separator, ...) _s #define strextend_with_separator(x, separator, ...) strextend_with_separator_internal(x, separator, __VA_ARGS__, NULL) #define strextend(x, ...) strextend_with_separator_internal(x, NULL, __VA_ARGS__, NULL) +char *strextendn(char **x, const char *s, size_t l); + int strextendf_with_separator(char **x, const char *separator, const char *format, ...) _printf_(3,4); #define strextendf(x, ...) strextendf_with_separator(x, NULL, __VA_ARGS__) diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c index e8fcc5a13c..6ec70054e7 100644 --- a/src/test/test-string-util.c +++ b/src/test/test-string-util.c @@ -1274,4 +1274,22 @@ TEST(version_is_valid) { assert_se(version_is_valid("6.2.12-300.fc38.x86_64")); } +TEST(strextendn) { + _cleanup_free_ char *x = NULL; + + assert_se(streq_ptr(strextendn(&x, NULL, 0), "")); + x = mfree(x); + + assert_se(streq_ptr(strextendn(&x, "", 0), "")); + x = mfree(x); + + assert_se(streq_ptr(strextendn(&x, "xxx", 3), "xxx")); + assert_se(streq_ptr(strextendn(&x, "xxx", 3), "xxxxxx")); + assert_se(streq_ptr(strextendn(&x, "...", 1), "xxxxxx.")); + assert_se(streq_ptr(strextendn(&x, "...", 2), "xxxxxx...")); + assert_se(streq_ptr(strextendn(&x, "...", 3), "xxxxxx......")); + assert_se(streq_ptr(strextendn(&x, "...", 4), "xxxxxx.........")); + x = mfree(x); +} + DEFINE_TEST_MAIN(LOG_DEBUG); From 2b07147e9c9d1f6ce032526a81961d005165614a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 26 Jun 2023 16:13:27 +0200 Subject: [PATCH 2/6] env-util: modernize env_name_is_valid_n() a bit If the size is specified as SIZE_MAX, then imply strlen(). --- src/basic/env-util.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/basic/env-util.c b/src/basic/env-util.c index 5a933d7f08..128102c688 100644 --- a/src/basic/env-util.c +++ b/src/basic/env-util.c @@ -26,20 +26,21 @@ "_" static bool env_name_is_valid_n(const char *e, size_t n) { - if (!e) - return false; + + if (n == SIZE_MAX) + n = strlen_ptr(e); if (n <= 0) return false; + assert(e); + if (ascii_isdigit(e[0])) 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. */ + /* 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 > (size_t) sysconf(_SC_ARG_MAX) - 2) return false; From 10930fbb1c0f4d25c8002267c5bcbc85615c45f3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 26 Jun 2023 16:14:27 +0200 Subject: [PATCH 3/6] env-util: add explicit size check before strndupa() in strv_env_get_n() Let's better be safe than sorry. --- src/basic/env-util.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/basic/env-util.c b/src/basic/env-util.c index 128102c688..8cc7b72511 100644 --- a/src/basic/env-util.c +++ b/src/basic/env-util.c @@ -514,6 +514,10 @@ char *strv_env_get_n(char **l, const char *name, size_t k, unsigned flags) { if (flags & REPLACE_ENV_USE_ENVIRONMENT) { const char *t; + /* Safety check that the name is not overly long, before we do a stack allocation */ + if (k > (size_t) sysconf(_SC_ARG_MAX) - 2) + return NULL; + t = strndupa_safe(name, k); return getenv(t); }; From d5ce24c99ea31a6d787b8e3ac92438cc1e2a27f0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 26 Jun 2023 17:10:28 +0200 Subject: [PATCH 4/6] env-util: introduce proper flags type ReplaceEnvFlags for replace_env() --- src/basic/env-util.c | 4 ++-- src/basic/env-util.h | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/basic/env-util.c b/src/basic/env-util.c index 8cc7b72511..f3b76d6ba2 100644 --- a/src/basic/env-util.c +++ b/src/basic/env-util.c @@ -500,7 +500,7 @@ int _strv_env_assign_many(char ***l, ...) { return 0; } -char *strv_env_get_n(char **l, const char *name, size_t k, unsigned flags) { +char *strv_env_get_n(char **l, const char *name, size_t k, ReplaceEnvFlags flags) { assert(name); if (k <= 0) @@ -578,7 +578,7 @@ char **strv_env_clean_with_callback(char **e, void (*invalid_callback)(const cha return e; } -char *replace_env_n(const char *format, size_t n, char **env, unsigned flags) { +char *replace_env_n(const char *format, size_t n, char **env, ReplaceEnvFlags flags) { enum { WORD, CURLY, diff --git a/src/basic/env-util.h b/src/basic/env-util.h index b0ff5a11d1..786ae4cbb0 100644 --- a/src/basic/env-util.h +++ b/src/basic/env-util.h @@ -19,19 +19,19 @@ bool env_name_is_valid(const char *e); bool env_value_is_valid(const char *e); bool env_assignment_is_valid(const char *e); -enum { +typedef enum ReplaceEnvFlags { REPLACE_ENV_USE_ENVIRONMENT = 1 << 0, REPLACE_ENV_ALLOW_BRACELESS = 1 << 1, REPLACE_ENV_ALLOW_EXTENDED = 1 << 2, -}; +} ReplaceEnvFlags; -char *replace_env_n(const char *format, size_t n, char **env, unsigned flags); -char **replace_env_argv(char **argv, char **env); - -static inline char *replace_env(const char *format, char **env, unsigned flags) { +char *replace_env_n(const char *format, size_t n, char **env, ReplaceEnvFlags flags); +static inline char *replace_env(const char *format, char **env, ReplaceEnvFlags flags) { return replace_env_n(format, strlen(format), env, flags); } +char **replace_env_argv(char **argv, char **env); + bool strv_env_is_valid(char **e); #define strv_env_clean(l) strv_env_clean_with_callback(l, NULL, NULL) char **strv_env_clean_with_callback(char **l, void (*invalid_callback)(const char *p, void *userdata), void *userdata); @@ -52,7 +52,7 @@ int strv_env_assign(char ***l, const char *key, const char *value); int _strv_env_assign_many(char ***l, ...) _sentinel_; #define strv_env_assign_many(l, ...) _strv_env_assign_many(l, __VA_ARGS__, NULL) -char *strv_env_get_n(char **l, const char *name, size_t k, unsigned flags) _pure_; +char *strv_env_get_n(char **l, const char *name, size_t k, ReplaceEnvFlags flags) _pure_; char *strv_env_get(char **x, const char *n) _pure_; char *strv_env_pairs_get(char **l, const char *name) _pure_; From 7658139c01395fdb83e1ba14023fb112cf220fc9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 26 Jun 2023 16:15:44 +0200 Subject: [PATCH 5/6] env-util: make strv_env_get() a static inline wrapper around strv_env_get_n() Let's make length = SIZE_MAX also mean "call strlen()". --- src/basic/env-util.c | 8 ++------ src/basic/env-util.h | 5 ++++- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/basic/env-util.c b/src/basic/env-util.c index f3b76d6ba2..701c9f9dd7 100644 --- a/src/basic/env-util.c +++ b/src/basic/env-util.c @@ -503,6 +503,8 @@ int _strv_env_assign_many(char ***l, ...) { char *strv_env_get_n(char **l, const char *name, size_t k, ReplaceEnvFlags flags) { assert(name); + if (k == SIZE_MAX) + k = strlen_ptr(name); if (k <= 0) return NULL; @@ -525,12 +527,6 @@ char *strv_env_get_n(char **l, const char *name, size_t k, ReplaceEnvFlags flags return NULL; } -char *strv_env_get(char **l, const char *name) { - assert(name); - - return strv_env_get_n(l, name, strlen(name), 0); -} - char *strv_env_pairs_get(char **l, const char *name) { char *result = NULL; diff --git a/src/basic/env-util.h b/src/basic/env-util.h index 786ae4cbb0..c3fc244798 100644 --- a/src/basic/env-util.h +++ b/src/basic/env-util.h @@ -53,7 +53,10 @@ int _strv_env_assign_many(char ***l, ...) _sentinel_; #define strv_env_assign_many(l, ...) _strv_env_assign_many(l, __VA_ARGS__, NULL) char *strv_env_get_n(char **l, const char *name, size_t k, ReplaceEnvFlags flags) _pure_; -char *strv_env_get(char **x, const char *n) _pure_; +static inline char *strv_env_get(char **x, const char *n) { + return strv_env_get_n(x, n, SIZE_MAX, 0); +} + char *strv_env_pairs_get(char **l, const char *name) _pure_; int getenv_bool(const char *p); From f331434d13488425ccd8485327085d15f8f92aea Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 26 Jun 2023 17:20:49 +0200 Subject: [PATCH 6/6] env-file: when resolving env vars in command lines, collect list of unset/invalid ones When resolving environment variables we currently silently resolve unset and invalid environment variables to empty strings. Let's do this slightly less silently: log about unset and invalid env vars, but still resolve them to an empty string. Fixes: #27036 --- src/basic/env-file.c | 13 +- src/basic/env-util.c | 248 ++++++++++++++++++++++++++++++--------- src/basic/env-util.h | 8 +- src/core/execute.c | 18 ++- src/run/run.c | 23 +++- src/test/test-env-util.c | 67 +++++++++-- 6 files changed, 290 insertions(+), 87 deletions(-) diff --git a/src/basic/env-file.c b/src/basic/env-file.c index eb5e640494..5fe045018d 100644 --- a/src/basic/env-file.c +++ b/src/basic/env-file.c @@ -525,6 +525,7 @@ static int merge_env_file_push( char ***env = ASSERT_PTR(userdata); char *expanded_value; + int r; assert(key); @@ -539,12 +540,12 @@ static int merge_env_file_push( return 0; } - expanded_value = replace_env(value, *env, - REPLACE_ENV_USE_ENVIRONMENT| - REPLACE_ENV_ALLOW_BRACELESS| - REPLACE_ENV_ALLOW_EXTENDED); - if (!expanded_value) - return -ENOMEM; + r = replace_env(value, + *env, + REPLACE_ENV_USE_ENVIRONMENT|REPLACE_ENV_ALLOW_BRACELESS|REPLACE_ENV_ALLOW_EXTENDED, + &expanded_value); + if (r < 0) + return log_error_errno(r, "%s:%u: Failed to expand variable '%s': %m", strna(filename), line, value); free_and_replace(value, expanded_value); diff --git a/src/basic/env-util.c b/src/basic/env-util.c index 701c9f9dd7..44772f778d 100644 --- a/src/basic/env-util.c +++ b/src/basic/env-util.c @@ -574,7 +574,61 @@ char **strv_env_clean_with_callback(char **e, void (*invalid_callback)(const cha return e; } -char *replace_env_n(const char *format, size_t n, char **env, ReplaceEnvFlags flags) { +static int strv_extend_with_length(char ***l, const char *s, size_t n) { + char *c; + + c = strndup(s, n); + if (!c) + return -ENOMEM; + + return strv_consume(l, c); +} + +static int strv_env_get_n_validated( + char **env, + const char *name, + size_t l, + ReplaceEnvFlags flags, + char **ret, /* points into the env block! do not free! */ + char ***unset_variables, /* updated in place */ + char ***bad_variables) { /* ditto */ + + char *e; + int r; + + assert(l == 0 || name); + assert(ret); + + if (env_name_is_valid_n(name, l)) { + e = strv_env_get_n(env, name, l, flags); + if (!e && unset_variables) { + r = strv_extend_with_length(unset_variables, name, l); + if (r < 0) + return r; + } + } else { + e = NULL; /* Resolve invalid variable names the same way as unset ones */ + + if (bad_variables) { + r = strv_extend_with_length(bad_variables, name, l); + if (r < 0) + return r; + } + } + + *ret = e; + return !!e; +} + +int replace_env_full( + const char *format, + size_t n, + char **env, + ReplaceEnvFlags flags, + char **ret, + char ***ret_unset_variables, + char ***ret_bad_variables) { + enum { WORD, CURLY, @@ -585,14 +639,21 @@ char *replace_env_n(const char *format, size_t n, char **env, ReplaceEnvFlags fl ALTERNATE_VALUE, } state = WORD; + _cleanup_strv_free_ char **unset_variables = NULL, **bad_variables = NULL; const char *e, *word = format, *test_value = NULL; /* test_value is initialized to appease gcc */ - char *k; _cleanup_free_ char *s = NULL; + char ***pu, ***pb, *k; size_t i, len = 0; /* len is initialized to appease gcc */ - int nest = 0; + int nest = 0, r; assert(format); + if (n == SIZE_MAX) + n = strlen(format); + + pu = ret_unset_variables ? &unset_variables : NULL; + pb = ret_bad_variables ? &bad_variables : NULL; + for (e = format, i = 0; *e && i < n; e ++, i ++) switch (state) { @@ -605,27 +666,28 @@ char *replace_env_n(const char *format, size_t n, char **env, ReplaceEnvFlags fl if (*e == '{') { k = strnappend(s, word, e-word-1); if (!k) - return NULL; + return -ENOMEM; free_and_replace(s, k); word = e-1; state = VARIABLE; nest++; + } else if (*e == '$') { k = strnappend(s, word, e-word); if (!k) - return NULL; + return -ENOMEM; free_and_replace(s, k); word = e+1; state = WORD; - } else if (flags & REPLACE_ENV_ALLOW_BRACELESS && strchr(VALID_BASH_ENV_NAME_CHARS, *e)) { + } else if (FLAGS_SET(flags, REPLACE_ENV_ALLOW_BRACELESS) && strchr(VALID_BASH_ENV_NAME_CHARS, *e)) { k = strnappend(s, word, e-word-1); if (!k) - return NULL; + return -ENOMEM; free_and_replace(s, k); @@ -638,12 +700,14 @@ char *replace_env_n(const char *format, size_t n, char **env, ReplaceEnvFlags fl case VARIABLE: if (*e == '}') { - const char *t; + char *t; - t = strv_env_get_n(env, word+2, e-word-2, flags); + r = strv_env_get_n_validated(env, word+2, e-word-2, flags, &t, pu, pb); + if (r < 0) + return r; if (!strextend(&s, t)) - return NULL; + return -ENOMEM; word = e+1; state = WORD; @@ -685,18 +749,37 @@ char *replace_env_n(const char *format, size_t n, char **env, ReplaceEnvFlags fl nest--; if (nest == 0) { - const char *t; + _cleanup_strv_free_ char **u = NULL, **b = NULL; _cleanup_free_ char *v = NULL; + char *t = NULL; - t = strv_env_get_n(env, word+2, len, flags); + r = strv_env_get_n_validated(env, word+2, len, flags, &t, pu, pb); + if (r < 0) + return r; - if (t && state == ALTERNATE_VALUE) - t = v = replace_env_n(test_value, e-test_value, env, flags); - else if (!t && state == DEFAULT_VALUE) - t = v = replace_env_n(test_value, e-test_value, env, flags); + if (t && state == ALTERNATE_VALUE) { + r = replace_env_full(test_value, e-test_value, env, flags, &v, pu ? &u : NULL, pb ? &b : NULL); + if (r < 0) + return r; + + t = v; + } else if (!t && state == DEFAULT_VALUE) { + r = replace_env_full(test_value, e-test_value, env, flags, &v, pu ? &u : NULL, pb ? &b : NULL); + if (r < 0) + return r; + + t = v; + } + + r = strv_extend_strv(&unset_variables, u, /* filter_duplicates= */ true); + if (r < 0) + return r; + r = strv_extend_strv(&bad_variables, b, /* filter_duplicates= */ true); + if (r < 0) + return r; if (!strextend(&s, t)) - return NULL; + return -ENOMEM; word = e+1; state = WORD; @@ -707,12 +790,14 @@ char *replace_env_n(const char *format, size_t n, char **env, ReplaceEnvFlags fl assert(flags & REPLACE_ENV_ALLOW_BRACELESS); if (!strchr(VALID_BASH_ENV_NAME_CHARS, *e)) { - const char *t; + char *t = NULL; - t = strv_env_get_n(env, word+1, e-word-1, flags); + r = strv_env_get_n_validated(env, word+1, e-word-1, flags, &t, &unset_variables, &bad_variables); + if (r < 0) + return r; if (!strextend(&s, t)) - return NULL; + return -ENOMEM; word = e--; i--; @@ -722,58 +807,83 @@ char *replace_env_n(const char *format, size_t n, char **env, ReplaceEnvFlags fl } if (state == VARIABLE_RAW) { - const char *t; + char *t; assert(flags & REPLACE_ENV_ALLOW_BRACELESS); - t = strv_env_get_n(env, word+1, e-word-1, flags); - return strjoin(s, t); - } else - return strnappend(s, word, e-word); + r = strv_env_get_n_validated(env, word+1, e-word-1, flags, &t, &unset_variables, &bad_variables); + if (r < 0) + return r; + + if (!strextend(&s, t)) + return -ENOMEM; + + } else if (!strextendn(&s, word, e-word)) + return -ENOMEM; + + if (ret_unset_variables) + *ret_unset_variables = TAKE_PTR(unset_variables); + if (ret_bad_variables) + *ret_bad_variables = TAKE_PTR(bad_variables); + + if (ret) + *ret = TAKE_PTR(s); + + return 0; } -char **replace_env_argv(char **argv, char **env) { - _cleanup_strv_free_ char **ret = NULL; +int replace_env_argv( + char **argv, + char **env, + char ***ret, + char ***ret_unset_variables, + char ***ret_bad_variables) { + + _cleanup_strv_free_ char **n = NULL, **unset_variables = NULL, **bad_variables = NULL; size_t k = 0, l = 0; + int r; l = strv_length(argv); - ret = new(char*, l+1); - if (!ret) - return NULL; + n = new(char*, l+1); + if (!n) + return -ENOMEM; STRV_FOREACH(i, argv) { + const char *word = *i; /* If $FOO appears as single word, replace it by the split up variable */ - if ((*i)[0] == '$' && !IN_SET((*i)[1], '{', '$')) { - char *e; - char **w; + if (word[0] == '$' && !IN_SET(word[1], '{', '$')) { _cleanup_strv_free_ char **m = NULL; + const char *name = word + 1; + char *e, **w; size_t q; - e = strv_env_get(env, *i+1); - if (e) { - int r; - - r = strv_split_full(&m, e, WHITESPACE, EXTRACT_RELAX|EXTRACT_UNQUOTE); - if (r < 0) { - ret[k] = NULL; - return NULL; - } - } + if (env_name_is_valid(name)) { + e = strv_env_get(env, name); + if (e) + r = strv_split_full(&m, e, WHITESPACE, EXTRACT_RELAX|EXTRACT_UNQUOTE); + else if (ret_unset_variables) + r = strv_extend(&unset_variables, name); + else + r = 0; + } else if (ret_bad_variables) + r = strv_extend(&bad_variables, name); + else + r = 0; + if (r < 0) + return r; q = strv_length(m); l = l + q - 1; - w = reallocarray(ret, l + 1, sizeof(char *)); - if (!w) { - ret[k] = NULL; - return NULL; - } + w = reallocarray(n, l + 1, sizeof(char*)); + if (!w) + return -ENOMEM; - ret = w; + n = w; if (m) { - memcpy(ret + k, m, q * sizeof(char*)); + memcpy(n + k, m, (q + 1) * sizeof(char*)); m = mfree(m); } @@ -781,15 +891,41 @@ char **replace_env_argv(char **argv, char **env) { continue; } + _cleanup_strv_free_ char **u = NULL, **b = NULL; + /* If ${FOO} appears as part of a word, replace it by the variable as-is */ - ret[k] = replace_env(*i, env, 0); - if (!ret[k]) - return NULL; - k++; + r = replace_env_full( + word, + /* length= */ SIZE_MAX, + env, + /* flags= */ 0, + n + k, + ret_unset_variables ? &u : NULL, + ret_bad_variables ? &b : NULL); + if (r < 0) + return r; + n[++k] = NULL; + + r = strv_extend_strv(&unset_variables, u, /* filter_duplicates= */ true); + if (r < 0) + return r; + + r = strv_extend_strv(&bad_variables, b, /*filter_duplicates= */ true); + if (r < 0) + return r; } - ret[k] = NULL; - return TAKE_PTR(ret); + if (ret_unset_variables) { + strv_uniq(strv_sort(unset_variables)); + *ret_unset_variables = TAKE_PTR(unset_variables); + } + if (ret_bad_variables) { + strv_uniq(strv_sort(bad_variables)); + *ret_bad_variables = TAKE_PTR(bad_variables); + } + + *ret = TAKE_PTR(n); + return 0; } int getenv_bool(const char *p) { diff --git a/src/basic/env-util.h b/src/basic/env-util.h index c3fc244798..ec3ac199ef 100644 --- a/src/basic/env-util.h +++ b/src/basic/env-util.h @@ -25,12 +25,12 @@ typedef enum ReplaceEnvFlags { REPLACE_ENV_ALLOW_EXTENDED = 1 << 2, } ReplaceEnvFlags; -char *replace_env_n(const char *format, size_t n, char **env, ReplaceEnvFlags flags); -static inline char *replace_env(const char *format, char **env, ReplaceEnvFlags flags) { - return replace_env_n(format, strlen(format), env, flags); +int replace_env_full(const char *format, size_t n, char **env, ReplaceEnvFlags flags, char **ret, char ***ret_unset_variables, char ***ret_bad_variables); +static inline int replace_env(const char *format, char **env, ReplaceEnvFlags flags, char **ret) { + return replace_env_full(format, SIZE_MAX, env, flags, ret, NULL, NULL); } -char **replace_env_argv(char **argv, char **env); +int replace_env_argv(char **argv, char **env, char ***ret, char ***ret_unset_variables, char ***ret_bad_variables); bool strv_env_is_valid(char **e); #define strv_env_clean(l) strv_env_clean_with_callback(l, NULL, NULL) diff --git a/src/core/execute.c b/src/core/execute.c index 9c0a2f4e8f..90b19b9e9b 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -5869,12 +5869,24 @@ static int exec_child( } if (!FLAGS_SET(command->flags, EXEC_COMMAND_NO_ENV_EXPAND)) { - replaced_argv = replace_env_argv(command->argv, accum_env); - if (!replaced_argv) { + _cleanup_strv_free_ char **unset_variables = NULL, **bad_variables = NULL; + + r = replace_env_argv(command->argv, accum_env, &replaced_argv, &unset_variables, &bad_variables); + if (r < 0) { *exit_status = EXIT_MEMORY; - return log_oom(); + return log_unit_error_errno(unit, r, "Failed to replace environment variables: %m"); } final_argv = replaced_argv; + + if (!strv_isempty(unset_variables)) { + _cleanup_free_ char *ju = strv_join(unset_variables, ", "); + log_unit_warning(unit, "Referenced but unset environment variable evaluates to an empty string: %s", strna(ju)); + } + + if (!strv_isempty(bad_variables)) { + _cleanup_free_ char *jb = strv_join(bad_variables, ", "); + log_unit_warning(unit, "Invalid environment variable name evaluates to an empty string: %s", strna(jb));; + } } else final_argv = command->argv; diff --git a/src/run/run.c b/src/run/run.c index 7dcd37cedd..7880a0ecc7 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -1472,7 +1472,7 @@ static int start_transient_scope(sd_bus *bus) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL, *reply = NULL; _cleanup_(bus_wait_for_jobs_freep) BusWaitForJobs *w = NULL; - _cleanup_strv_free_ char **env = NULL, **user_env = NULL, **expanded_cmdline = NULL; + _cleanup_strv_free_ char **env = NULL, **user_env = NULL; _cleanup_free_ char *scope = NULL; const char *object = NULL; sd_id128_t invocation_id; @@ -1615,10 +1615,23 @@ static int start_transient_scope(sd_bus *bus) { log_info("Running scope as unit: %s", scope); if (arg_expand_environment) { - expanded_cmdline = replace_env_argv(arg_cmdline, env); - if (!expanded_cmdline) - return log_oom(); - arg_cmdline = expanded_cmdline; + _cleanup_strv_free_ char **expanded_cmdline = NULL, **unset_variables = NULL, **bad_variables = NULL; + + r = replace_env_argv(arg_cmdline, env, &expanded_cmdline, &unset_variables, &bad_variables); + if (r < 0) + return log_error_errno(r, "Failed to expand environment variables: %m"); + + free_and_replace(arg_cmdline, expanded_cmdline); + + if (!strv_isempty(unset_variables)) { + _cleanup_free_ char *ju = strv_join(unset_variables, ", "); + log_warning("Referenced but unset environment variable evaluates to an empty string: %s", strna(ju)); + } + + if (!strv_isempty(bad_variables)) { + _cleanup_free_ char *jb = strv_join(bad_variables, ", "); + log_warning("Invalid environment variable name evaluates to an empty string: %s", strna(jb)); + } } execvpe(arg_cmdline[0], arg_cmdline, env); diff --git a/src/test/test-env-util.c b/src/test/test-env-util.c index 0f58d2fed0..a6b5f2f347 100644 --- a/src/test/test-env-util.c +++ b/src/test/test-env-util.c @@ -200,19 +200,19 @@ static void test_replace_env1(bool braceless) { _cleanup_free_ char *t = NULL, *s = NULL, *q = NULL, *r = NULL, *p = NULL; unsigned flags = REPLACE_ENV_ALLOW_BRACELESS*braceless; - t = replace_env("FOO=$FOO=${FOO}", (char**) env, flags); + assert_se(replace_env("FOO=$FOO=${FOO}", (char**) env, flags, &t) >= 0); assert_se(streq(t, braceless ? "FOO=BAR BAR=BAR BAR" : "FOO=$FOO=BAR BAR")); - s = replace_env("BAR=$BAR=${BAR}", (char**) env, flags); + assert_se(replace_env("BAR=$BAR=${BAR}", (char**) env, flags, &s) >= 0); assert_se(streq(s, braceless ? "BAR=waldo=waldo" : "BAR=$BAR=waldo")); - q = replace_env("BARBAR=$BARBAR=${BARBAR}", (char**) env, flags); + assert_se(replace_env("BARBAR=$BARBAR=${BARBAR}", (char**) env, flags, &q) >= 0); assert_se(streq(q, braceless ? "BARBAR==" : "BARBAR=$BARBAR=")); - r = replace_env("BAR=$BAR$BAR${BAR}${BAR}", (char**) env, flags); + assert_se(replace_env("BAR=$BAR$BAR${BAR}${BAR}", (char**) env, flags, &r) >= 0); assert_se(streq(r, braceless ? "BAR=waldowaldowaldowaldo" : "BAR=$BAR$BARwaldowaldo")); - p = replace_env("${BAR}$BAR$BAR", (char**) env, flags); + assert_se(replace_env("${BAR}$BAR$BAR", (char**) env, flags, &p) >= 0); assert_se(streq(p, braceless ? "waldowaldowaldo" : "waldo$BAR$BAR")); } @@ -227,25 +227,25 @@ static void test_replace_env2(bool extended) { _cleanup_free_ char *t = NULL, *s = NULL, *q = NULL, *r = NULL, *p = NULL, *x = NULL, *y = NULL; unsigned flags = REPLACE_ENV_ALLOW_EXTENDED*extended; - t = replace_env("FOO=${FOO:-${BAR}}", (char**) env, flags); + assert_se(replace_env("FOO=${FOO:-${BAR}}", (char**) env, flags, &t) >= 0); assert_se(streq(t, extended ? "FOO=foo" : "FOO=${FOO:-bar}")); - s = replace_env("BAR=${XXX:-${BAR}}", (char**) env, flags); + assert_se(replace_env("BAR=${XXX:-${BAR}}", (char**) env, flags, &s) >= 0); assert_se(streq(s, extended ? "BAR=bar" : "BAR=${XXX:-bar}")); - q = replace_env("XXX=${XXX:+${BAR}}", (char**) env, flags); + assert_se(replace_env("XXX=${XXX:+${BAR}}", (char**) env, flags, &q) >= 0); assert_se(streq(q, extended ? "XXX=" : "XXX=${XXX:+bar}")); - r = replace_env("FOO=${FOO:+${BAR}}", (char**) env, flags); + assert_se(replace_env("FOO=${FOO:+${BAR}}", (char**) env, flags, &r) >= 0); assert_se(streq(r, extended ? "FOO=bar" : "FOO=${FOO:+bar}")); - p = replace_env("FOO=${FOO:-${BAR}post}", (char**) env, flags); + assert_se(replace_env("FOO=${FOO:-${BAR}post}", (char**) env, flags, &p) >= 0); assert_se(streq(p, extended ? "FOO=foo" : "FOO=${FOO:-barpost}")); - x = replace_env("XXX=${XXX:+${BAR}post}", (char**) env, flags); + assert_se(replace_env("XXX=${XXX:+${BAR}post}", (char**) env, flags, &x) >= 0); assert_se(streq(x, extended ? "XXX=" : "XXX=${XXX:+barpost}")); - y = replace_env("FOO=${FOO}between${BAR:-baz}", (char**) env, flags); + assert_se(replace_env("FOO=${FOO}between${BAR:-baz}", (char**) env, flags, &y) >= 0); assert_se(streq(y, extended ? "FOO=foobetweenbar" : "FOO=foobetween${BAR:-baz}")); } @@ -284,7 +284,7 @@ TEST(replace_env_argv) { }; _cleanup_strv_free_ char **r = NULL; - r = replace_env_argv((char**) line, (char**) env); + assert_se(replace_env_argv((char**) line, (char**) env, &r, NULL, NULL) >= 0); assert_se(r); assert_se(streq(r[0], "FOO$FOO")); assert_se(streq(r[1], "FOO$FOOFOO")); @@ -306,6 +306,47 @@ TEST(replace_env_argv) { assert_se(strv_length(r) == 17); } +TEST(replace_env_argv_bad) { + + const char *env[] = { + "FOO=BAR BAR", + "BAR=waldo", + NULL + }; + + const char *line[] = { + "$FOO", + "A${FOO}B", + "a${~}${%}b", + "x${}y", + "$UNSET2", + "z${UNSET3}z${UNSET1}z", + "piff${UNSET2}piff", + NULL + }; + + _cleanup_strv_free_ char **bad = NULL, **unset = NULL, **replaced = NULL; + + assert_se(replace_env_argv((char**) line, (char**) env, &replaced, &unset, &bad) >= 0); + + assert_se(strv_equal(replaced, STRV_MAKE( + "BAR", + "BAR", + "ABAR BARB", + "ab", + "xy", + "zzz", + "piffpiff"))); + + assert_se(strv_equal(unset, STRV_MAKE( + "UNSET1", + "UNSET2", + "UNSET3"))); + assert_se(strv_equal(bad, STRV_MAKE("", + "%", + "~"))); +} + TEST(env_clean) { _cleanup_strv_free_ char **e = strv_new("FOOBAR=WALDO", "FOOBAR=WALDO",