shaba/systemd
1
0
Fork 0
mirror of https://github.com/systemd/systemd.git synced 2025-01-12 13:18:14 +03:00
Code

core/unit: fix shell-escaping of strings

Browse Source 
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.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2023-04-03 12:43:53 +02:00
parent a12bc99ef0
commit e7416db183
2 changed files with 17 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
src/core/unit.c
View File

@ -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);

10
src/test/test-core-unit.c
View File

@ -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\"",