core/unit: fix shell-escaping of strings

Our escaping of '$' is '$$', not '\$'. We would write unit files that
were not valid:
  $ systemd-run --user bash -c 'echo $$; sleep 1000'
  Running as unit: run-r1c7c45b5b69f487c86ae205e12100808.service
  $ systemctl cat --user run-r1c7c45b5b69f487c86ae205e12100808
  # /run/user/1000/systemd/transient/run-r1c7c45b5b69f487c86ae205e12100808.service
  ...
  ExecStart="/usr/bin/bash" "-c" "echo \$\$\; sleep 1000"

  $ systemd-analyze verify /run/user/1000/systemd/transient/run-r1c7c45b5b69f487c86ae205e12100808.service
  /run/user/1000/systemd/transient/run-r1c7c45b5b69f487c86ae205e12100808.service:7:
    Ignoring unknown escape sequences: "echo \$\$\; sleep 1000"

Similarly, ';' cannot be escaped as '\;'. Only a handful of characters
listed in "Supported escapes" is allowed.

Escaping of "'" can be done, but it's not useful because we use double quotes
around the string anyway whenever we do escaping.

unit_write_setting() is called all over the place. In a great majority of
places we write either fixed strings or something that we generate ourselves,
so no escaping or quoting is needed. (And it's not allowed, e.g.
'Type="oneshot"' would not work.)  But if we forgot to add escaping or quoting
for a free-style string, it would probably allow writing a unit file that would
be read completely wrong. I looked over various places where
unit_write_setting() is called, and I couldn't find any place where
quoting/escaping was forgotten. But trying to figure out the full
ramifications of this change is not easy.

2 files changed, 17 insertions, 8 deletions

diff --git a/src/core/unit.c b/src/core/unit.c
index 846d15b415..2d47d9de8c 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -4332,10 +4332,17 @@ const char* unit_escape_setting(const char *s, UnitWriteFlags flags, char **buf)
         }
 
         /* We either do C-escaping or shell-escaping, to additionally escape characters that we parse for
-         * ExecStart= and friends, i.e. '$' and ';' and quotes. */
+         * ExecStart= and friends, i.e. '$' and quotes. */
 
         if (flags & UNIT_ESCAPE_EXEC_SYNTAX) {
-                char *t2 = shell_escape(s, "$;'\"");
+                char *t2;
+
+                t2 = strreplace(s, "$", "$$");
+                if (!t2)
+                        return NULL;
+                free_and_replace(t, t2);
+
+                t2 = shell_escape(t, "\"");
                 if (!t2)
                         return NULL;
                 free_and_replace(t, t2);
@@ -4343,7 +4350,9 @@ const char* unit_escape_setting(const char *s, UnitWriteFlags flags, char **buf)
                 s = t;
 
         } else if (flags & UNIT_ESCAPE_C) {
-                char *t2 = cescape(s);
+                char *t2;
+
+                t2 = cescape(s);
                 if (!t2)
                         return NULL;
                 free_and_replace(t, t2);
diff --git a/src/test/test-core-unit.c b/src/test/test-core-unit.c
index 6593f2fb4b..ea514a5c8c 100644
--- a/src/test/test-core-unit.c
+++ b/src/test/test-core-unit.c
@@ -41,13 +41,13 @@ static void test_unit_escape_setting_one(
 
 TEST(unit_escape_setting) {
         test_unit_escape_setting_one("/sbin/sbash", NULL, NULL);
-        test_unit_escape_setting_one("$", "\\$", "$");
-        test_unit_escape_setting_one("$$", "\\$\\$", "$$");
-        test_unit_escape_setting_one("'", "\\'", NULL);
+        test_unit_escape_setting_one("$", "$$", "$");
+        test_unit_escape_setting_one("$$", "$$$$", "$$");
+        test_unit_escape_setting_one("'", "'", "\\'");
         test_unit_escape_setting_one("\"", "\\\"", NULL);
         test_unit_escape_setting_one("\t", "\\t", NULL);
         test_unit_escape_setting_one(" ", NULL, NULL);
-        test_unit_escape_setting_one("$;'\"\t\n", "\\$\\;\\'\\\"\\t\\n", "$;\\'\\\"\\t\\n");
+        test_unit_escape_setting_one("$;'\"\t\n", "$$;'\\\"\\t\\n", "$;\\'\\\"\\t\\n");
 }
 
 static void test_unit_concat_strv_one(
@@ -89,7 +89,7 @@ TEST(unit_concat_strv) {
                                   NULL);
         test_unit_concat_strv_one(STRV_MAKE("a", " ", "$", "$$", ""),
                                   "\"a\" \" \" \"$\" \"$$\" \"\"",
-                                  "\"a\" \" \" \"\\$\" \"\\$\\$\" \"\"",
+                                  "\"a\" \" \" \"$$\" \"$$$$\" \"\"",
                                   NULL);
         test_unit_concat_strv_one(STRV_MAKE("\n", " ", "\t"),
                                   "\"\n\" \" \" \"\t\"",