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 2302992fa5b461afb235dc85ee7d366c744d131e..4022c25c3631e29e0c79c01af217a9e50fffdbcd 100644 (file)
--- 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 207fac8dc1369ca7393caab900595a26b2170d10..aced9e8e3db80004d2b5d398ed36640bf032e3a2 100644 (file)
--- 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 3c2dab18558980c5094c1aba7976c41da0649802..8185f67e004bdeeaec654f951d7a42caa4ba80cc 100644 (file)
--- 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 94fe042aa984f7a1263bff791d881502b2242c4a..482b0751b938519509b92e1136d289946b5f0e56 100644 (file)
--- 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 c204cbae22b05e89bdaf6ef8daef27a8b5397ec7..b117335db88555175ca4b8579f814fa700c734b8 100644 (file)
--- 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();