From: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Date: Sat, 18 Feb 2017 03:56:28 +0000 (-0500)
Subject: Tighten checking for variable validity
X-Git-Tag: v233~59^2~4
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=184d19047370652184a44686cf85bf5001bdf413;p=thirdparty%2Fsystemd.git

Tighten checking for variable validity

In the future we might want to allow additional syntax (for example
"unset VAR". But let's check that the data we're getting does not contain
anything unexpected.
---

diff --git a/man/environment.d.xml b/man/environment.d.xml
index 2302992fa5b..4022c25c363 100644
--- a/man/environment.d.xml
+++ b/man/environment.d.xml
@@ -81,6 +81,9 @@
     and <literal>$OTHER_KEY</literal> format. No other elements of shell syntax are supported.
     </para>
 
+    <para>Each<replaceable>KEY</replaceable> must be a valid variable name. Empty lines
+    and lines beginning with the comment character <literal>#</literal> are ignored.</para>
+
     <refsect2>
       <title>Example</title>
       <example>
diff --git a/src/basic/exec-util.c b/src/basic/exec-util.c
index 207fac8dc13..aced9e8e3db 100644
--- a/src/basic/exec-util.c
+++ b/src/basic/exec-util.c
@@ -279,6 +279,11 @@ static int gather_environment_generate(int fd, void *arg) {
         STRV_FOREACH_PAIR(x, y, new) {
                 char *p;
 
+                if (!env_name_is_valid(*x)) {
+                        log_warning("Invalid variable assignment \"%s=...\", ignoring.", *x);
+                        continue;
+                }
+
                 p = strjoin(*x, "=", *y);
                 if (!p)
                         return -ENOMEM;
diff --git a/src/basic/fileio.c b/src/basic/fileio.c
index 3c2dab18558..8185f67e004 100644
--- a/src/basic/fileio.c
+++ b/src/basic/fileio.c
@@ -773,6 +773,16 @@ static int merge_env_file_push(
 
         assert(env);
 
+        if (!value) {
+                log_error("%s:%u: invalid syntax (around \"%s\"), ignoring.", strna(filename), line, key);
+                return 0;
+        }
+
+        if (!env_name_is_valid(key)) {
+                log_error("%s:%u: invalid variable name \"%s\", ignoring.", strna(filename), line, key);
+                return 0;
+        }
+
         expanded_value = replace_env(value, *env,
                                      REPLACE_ENV_USE_ENVIRONMENT|REPLACE_ENV_ALLOW_BRACELESS);
         if (!expanded_value)
diff --git a/src/test/test-exec-util.c b/src/test/test-exec-util.c
index 94fe042aa98..482b0751b93 100644
--- a/src/test/test-exec-util.c
+++ b/src/test/test-exec-util.c
@@ -305,8 +305,16 @@ static void test_environment_gathering(void) {
                                     "echo A=$A:24\n"
                                     "echo B=12\n"
                                     "echo C=000\n"
-                                    "echo C=001\n"                   /* variable overwriting */
-                                    "echo PATH=$PATH:/no/such/file", /* variable from manager */
+                                    "echo C=001\n"                    /* variable overwriting */
+                                     /* various invalid entries */
+                                    "echo unset A\n"
+                                    "echo unset A=\n"
+                                    "echo unset A=B\n"
+                                    "echo unset \n"
+                                    "echo A B=C\n"
+                                    "echo A\n"
+                                    /* test variable assignment without newline */
+                                    "echo PATH=$PATH:/no/such/file",   /* no newline */
                                     WRITE_STRING_FILE_CREATE) == 0);
 
         assert_se(chmod(name, 0755) == 0);
diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c
index c204cbae22b..b117335db88 100644
--- a/src/test/test-fileio.c
+++ b/src/test/test-fileio.c
@@ -263,6 +263,45 @@ static void test_merge_env_file(void) {
         assert_se(a[6] == NULL);
 }
 
+static void test_merge_env_file_invalid(void) {
+        char t[] = "/tmp/test-fileio-XXXXXX";
+        int fd, r;
+        FILE *f;
+        _cleanup_strv_free_ char **a = NULL;
+        char **i;
+
+        fd = mkostemp_safe(t);
+        assert_se(fd >= 0);
+
+        log_info("/* %s (%s) */", __func__, t);
+
+        f = fdopen(fd, "w");
+        assert_se(f);
+
+        r = write_string_stream(f,
+                                "unset one   \n"
+                                "unset one=   \n"
+                                "unset one=1   \n"
+                                "one   \n"
+                                "one =  \n"
+                                "one two =\n"
+                                "\x20two=\n"
+                                "#comment=comment\n"
+                                ";comment2=comment2\n"
+                                "#\n"
+                                "\n\n"                  /* empty line */
+                                , false);
+        assert(r >= 0);
+
+        r = merge_env_file(&a, NULL, t);
+        assert_se(r >= 0);
+
+        STRV_FOREACH(i, a)
+                log_info("Got: <%s>", *i);
+
+        assert_se(strv_isempty(a));
+}
+
 static void test_executable_is_script(void) {
         char t[] = "/tmp/test-executable-XXXXXX";
         int fd, r;
@@ -620,6 +659,7 @@ int main(int argc, char *argv[]) {
         test_parse_env_file();
         test_parse_multiline_env_file();
         test_merge_env_file();
+        test_merge_env_file_invalid();
         test_executable_is_script();
         test_status_field();
         test_capeff();