generators: be more careful when writing unit settings that support specifier expansion

Let's always escape strings we receive from the user before writing them
out to unit file settings that suppor specifier expansion, so that user
strings are transported as-is.

diff --git a/src/cryptsetup/cryptsetup-generator.c b/src/cryptsetup/cryptsetup-generator.c
index d0be412..e2dc96b 100644 (file)
--- a/src/cryptsetup/cryptsetup-generator.c
+++ b/src/cryptsetup/cryptsetup-generator.c
@@ -32,6 +32,7 @@
 #include "parse-util.h"
 #include "path-util.h"
 #include "proc-cmdline.h"
+#include "specifier.h"
 #include "string-util.h"
 #include "strv.h"
 #include "unit-name.h"
@@ -60,7 +61,7 @@ static int create_disk(
                 const char *options) {
 
         _cleanup_free_ char *p = NULL, *n = NULL, *d = NULL, *u = NULL, *e = NULL,
-                *filtered = NULL;
+                *filtered = NULL, *u_escaped = NULL, *password_escaped = NULL, *filtered_escaped = NULL, *name_escaped = NULL;
         _cleanup_fclose_ FILE *f = NULL;
         const char *dmname;
         bool noauto, nofail, tmp, swap, netdev;
@@ -80,6 +81,10 @@ static int create_disk(
                 return -EINVAL;
         }
 
+        name_escaped = specifier_escape(name);
+        if (!name_escaped)
+                return log_oom();
+
         e = unit_name_escape(name);
         if (!e)
                 return log_oom();
@@ -96,10 +101,18 @@ static int create_disk(
         if (!u)
                 return log_oom();
 
+        u_escaped = specifier_escape(u);
+        if (!u_escaped)
+                return log_oom();
+
         r = unit_name_from_path(u, ".device", &d);
         if (r < 0)
                 return log_error_errno(r, "Failed to generate unit name: %m");
 
+        password_escaped = specifier_escape(password);
+        if (!password_escaped)
+                return log_oom();
+
         f = fopen(p, "wxe");
         if (!f)
                 return log_error_errno(errno, "Failed to create unit file %s: %m", p);
@@ -142,7 +155,7 @@ static int create_disk(
 
                                         fprintf(f, "After=%1$s\nRequires=%1$s\n", dd);
                                 } else
-                                        fprintf(f, "RequiresMountsFor=%s\n", password);
+                                        fprintf(f, "RequiresMountsFor=%s\n", password_escaped);
                         }
                 }
         }
@@ -160,12 +173,17 @@ static int create_disk(
         } else
                 fprintf(f,
                         "RequiresMountsFor=%s\n",
-                        u);
+                        u_escaped);
+
 
         r = generator_write_timeouts(arg_dest, device, name, options, &filtered);
         if (r < 0)
                 return r;
 
+        filtered_escaped = specifier_escape(filtered);
+        if (!filtered_escaped)
+                return log_oom();
+
         fprintf(f,
                 "\n[Service]\n"
                 "Type=oneshot\n"
@@ -174,18 +192,18 @@ static int create_disk(
                 "KeyringMode=shared\n" /* make sure we can share cached keys among instances */
                 "ExecStart=" SYSTEMD_CRYPTSETUP_PATH " attach '%s' '%s' '%s' '%s'\n"
                 "ExecStop=" SYSTEMD_CRYPTSETUP_PATH " detach '%s'\n",
-                name, u, strempty(password), strempty(filtered),
-                name);
+                name_escaped, u_escaped, strempty(password_escaped), strempty(filtered_escaped),
+                name_escaped);
 
         if (tmp)
                 fprintf(f,
                         "ExecStartPost=/sbin/mke2fs '/dev/mapper/%s'\n",
-                        name);
+                        name_escaped);
 
         if (swap)
                 fprintf(f,
                         "ExecStartPost=/sbin/mkswap '/dev/mapper/%s'\n",
-                        name);
+                        name_escaped);
 
         r = fflush_and_check(f);
         if (r < 0)

diff --git a/src/fstab-generator/fstab-generator.c b/src/fstab-generator/fstab-generator.c
index 243c116..6d6895a 100644 (file)
--- a/src/fstab-generator/fstab-generator.c
+++ b/src/fstab-generator/fstab-generator.c
@@ -26,8 +26,8 @@
 
 #include "alloc-util.h"
 #include "fd-util.h"
-#include "fs-util.h"
 #include "fileio.h"
+#include "fs-util.h"
 #include "fstab-util.h"
 #include "generator.h"
 #include "log.h"
@@ -38,6 +38,7 @@
 #include "path-util.h"
 #include "proc-cmdline.h"
 #include "special.h"
+#include "specifier.h"
 #include "stat-util.h"
 #include "string-util.h"
 #include "strv.h"
@@ -68,7 +69,7 @@ static int write_options(FILE *f, const char *options) {
         if (streq(options, "defaults"))
                 return 0;
 
-        o = strreplace(options, "%", "%%");
+        o = specifier_escape(options);
         if (!o)
                 return log_oom();
 
@@ -79,7 +80,7 @@ static int write_options(FILE *f, const char *options) {
 static int write_what(FILE *f, const char *what) {
         _cleanup_free_ char *w = NULL;
 
-        w = strreplace(what, "%", "%%");
+        w = specifier_escape(what);
         if (!w)
                 return log_oom();
 
@@ -262,7 +263,7 @@ static int write_before(FILE *f, const char *opts) {
 }
 
 static int write_requires_mounts_for(FILE *f, const char *opts) {
-        _cleanup_strv_free_ char **paths = NULL;
+        _cleanup_strv_free_ char **paths = NULL, **paths_escaped = NULL;
         _cleanup_free_ char *res = NULL;
         int r;
 
@@ -275,7 +276,11 @@ static int write_requires_mounts_for(FILE *f, const char *opts) {
         if (r == 0)
                 return 0;
 
-        res = strv_join(paths, " ");
+        r = specifier_escape_strv(paths, &paths_escaped);
+        if (r < 0)
+                return log_error_errno(r, "Failed to escape paths: %m");
+
+        res = strv_join(paths_escaped, " ");
         if (!res)
                 return log_oom();
 
@@ -301,7 +306,8 @@ static int add_mount(
         _cleanup_free_ char
                 *name = NULL, *unit = NULL,
                 *automount_name = NULL, *automount_unit = NULL,
-                *filtered = NULL;
+                *filtered = NULL,
+                *where_escaped = NULL;
         _cleanup_fclose_ FILE *f = NULL;
         int r;
 
@@ -398,14 +404,25 @@ static int add_mount(
         fprintf(f, "\n[Mount]\n");
         if (original_where)
                 fprintf(f, "# Canonicalized from %s\n", original_where);
-        fprintf(f, "Where=%s\n", where);
+
+        where_escaped = specifier_escape(where);
+        if (!where_escaped)
+                return log_oom();
+        fprintf(f, "Where=%s\n", where_escaped);
 
         r = write_what(f, what);
         if (r < 0)
                 return r;
 
-        if (!isempty(fstype) && !streq(fstype, "auto"))
-                fprintf(f, "Type=%s\n", fstype);
+        if (!isempty(fstype) && !streq(fstype, "auto")) {
+                _cleanup_free_ char *t;
+
+                t = specifier_escape(fstype);
+                if (!t)
+                        return -ENOMEM;
+
+                fprintf(f, "Type=%s\n", t);
+        }
 
         r = generator_write_timeouts(dest, what, where, opts, &filtered);
         if (r < 0)
@@ -476,7 +493,7 @@ static int add_mount(
                         "\n"
                         "[Automount]\n"
                         "Where=%s\n",
-                        where);
+                        where_escaped);
 
                 r = write_idle_timeout(f, where, opts);
                 if (r < 0)

diff --git a/src/gpt-auto-generator/gpt-auto-generator.c b/src/gpt-auto-generator/gpt-auto-generator.c
index 7d014db..9e8b956 100644 (file)
--- a/src/gpt-auto-generator/gpt-auto-generator.c
+++ b/src/gpt-auto-generator/gpt-auto-generator.c
@@ -44,6 +44,7 @@
 #include "path-util.h"
 #include "proc-cmdline.h"
 #include "special.h"
+#include "specifier.h"
 #include "stat-util.h"
 #include "string-util.h"
 #include "udev-util.h"
@@ -57,7 +58,7 @@ static bool arg_root_enabled = true;
 static bool arg_root_rw = false;
 
 static int add_cryptsetup(const char *id, const char *what, bool rw, bool require, char **device) {
-        _cleanup_free_ char *e = NULL, *n = NULL, *p = NULL, *d = NULL;
+        _cleanup_free_ char *e = NULL, *n = NULL, *p = NULL, *d = NULL, *id_escaped = NULL, *what_escaped = NULL;
         _cleanup_fclose_ FILE *f = NULL;
         char *ret;
         int r;
@@ -77,6 +78,14 @@ static int add_cryptsetup(const char *id, const char *what, bool rw, bool requir
         if (r < 0)
                 return log_error_errno(r, "Failed to generate unit name: %m");
 
+        id_escaped = specifier_escape(id);
+        if (!id_escaped)
+                return log_oom();
+
+        what_escaped = specifier_escape(what);
+        if (!what_escaped)
+                return log_oom();
+
         p = strjoin(arg_dest, "/", n);
         if (!p)
                 return log_oom();
@@ -104,8 +113,8 @@ static int add_cryptsetup(const char *id, const char *what, bool rw, bool requir
                 "ExecStart=" SYSTEMD_CRYPTSETUP_PATH " attach '%s' '%s' '' '%s'\n"
                 "ExecStop=" SYSTEMD_CRYPTSETUP_PATH " detach '%s'\n",
                 d, d,
-                id, what, rw ? "" : "read-only",
-                id);
+                id_escaped, what_escaped, rw ? "" : "read-only",
+                id_escaped);
 
         r = fflush_and_check(f);
         if (r < 0)
@@ -165,6 +174,10 @@ static int add_mount(
         _cleanup_fclose_ FILE *f = NULL;
         int r;
 
+        /* Note that we don't apply specifier escaping on the input strings here, since we know they are not configured
+         * externally, but all originate from our own sources here, and hence we know they contain no % characters that
+         * could potentially be understood as specifiers. */
+
         assert(id);
         assert(what);
         assert(where);

diff --git a/src/shared/generator.c b/src/shared/generator.c
index a967021..462457e 100644 (file)
--- a/src/shared/generator.c
+++ b/src/shared/generator.c
@@ -33,6 +33,7 @@
 #include "mkdir.h"
 #include "path-util.h"
 #include "special.h"
+#include "specifier.h"
 #include "string-util.h"
 #include "time-util.h"
 #include "unit-name.h"
@@ -55,15 +56,19 @@ int generator_add_symlink(const char *root, const char *dst, const char *dep_typ
 }
 
 static int write_fsck_sysroot_service(const char *dir, const char *what) {
-        _cleanup_free_ char *device = NULL, *escaped = NULL;
+        _cleanup_free_ char *device = NULL, *escaped = NULL, *escaped2 = NULL;
         _cleanup_fclose_ FILE *f = NULL;
         const char *unit;
         int r;
 
-        escaped = cescape(what);
+        escaped = specifier_escape(what);
         if (!escaped)
                 return log_oom();
 
+        escaped2 = cescape(escaped);
+        if (!escaped2)
+                return log_oom();
+
         unit = strjoina(dir, "/systemd-fsck-root.service");
         log_debug("Creating %s", unit);
 
@@ -91,9 +96,9 @@ static int write_fsck_sysroot_service(const char *dir, const char *what) {
                 "ExecStart=" SYSTEMD_FSCK_PATH " %4$s\n"
                 "TimeoutSec=0\n",
                 program_invocation_short_name,
-                what,
+                escaped,
                 device,
-                escaped);
+                escaped2);
 
         r = fflush_and_check(f);
         if (r < 0)

diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c
index f59c277..51306b5 100644 (file)
--- a/src/sysv-generator/sysv-generator.c
+++ b/src/sysv-generator/sysv-generator.c
@@ -39,6 +39,7 @@
 #include "path-util.h"
 #include "set.h"
 #include "special.h"
+#include "specifier.h"
 #include "stat-util.h"
 #include "string-util.h"
 #include "strv.h"
@@ -119,6 +120,7 @@ static int add_alias(const char *service, const char *alias) {
 }
 
 static int generate_unit_file(SysvStub *s) {
+        _cleanup_free_ char *path_escaped = NULL;
         _cleanup_fclose_ FILE *f = NULL;
         const char *unit;
         char **p;
@@ -129,6 +131,10 @@ static int generate_unit_file(SysvStub *s) {
         if (!s->loaded)
                 return 0;
 
+        path_escaped = specifier_escape(s->path);
+        if (!path_escaped)
+                return log_oom();
+
         unit = strjoina(arg_dest, "/", s->name);
 
         /* We might already have a symlink with the same name from a Provides:,
@@ -148,10 +154,17 @@ static int generate_unit_file(SysvStub *s) {
                 "[Unit]\n"
                 "Documentation=man:systemd-sysv-generator(8)\n"
                 "SourcePath=%s\n",
-                s->path);
+                path_escaped);
+
+        if (s->description) {
+                _cleanup_free_ char *t;
+
+                t = specifier_escape(s->description);
+                if (!t)
+                        return log_oom();
 
-        if (s->description)
-                fprintf(f, "Description=%s\n", s->description);
+                fprintf(f, "Description=%s\n", t);
+        }
 
         STRV_FOREACH(p, s->before)
                 fprintf(f, "Before=%s\n", *p);
@@ -171,8 +184,15 @@ static int generate_unit_file(SysvStub *s) {
                 "RemainAfterExit=%s\n",
                 yes_no(!s->pid_file));
 
-        if (s->pid_file)
-                fprintf(f, "PIDFile=%s\n", s->pid_file);
+        if (s->pid_file) {
+                _cleanup_free_ char *t;
+
+                t = specifier_escape(s->pid_file);
+                if (!t)
+                        return log_oom();
+
+                fprintf(f, "PIDFile=%s\n", t);
+        }
 
         /* Consider two special LSB exit codes a clean exit */
         if (s->has_lsb)
@@ -184,10 +204,10 @@ static int generate_unit_file(SysvStub *s) {
         fprintf(f,
                 "ExecStart=%s start\n"
                 "ExecStop=%s stop\n",
-                s->path, s->path);
+                path_escaped, path_escaped);
 
         if (s->reload)
-                fprintf(f, "ExecReload=%s reload\n", s->path);
+                fprintf(f, "ExecReload=%s reload\n", path_escaped);
 
         r = fflush_and_check(f);
         if (r < 0)

diff --git a/src/veritysetup/veritysetup-generator.c b/src/veritysetup/veritysetup-generator.c
index ef43aed..5919b13 100644 (file)
--- a/src/veritysetup/veritysetup-generator.c
+++ b/src/veritysetup/veritysetup-generator.c
@@ -32,6 +32,7 @@
 #include "mkdir.h"
 #include "parse-util.h"
 #include "proc-cmdline.h"
+#include "specifier.h"
 #include "string-util.h"
 #include "unit-name.h"
 
@@ -42,7 +43,7 @@ static char *arg_data_what = NULL;
 static char *arg_hash_what = NULL;
 
 static int create_device(void) {
-        _cleanup_free_ char *u = NULL, *v = NULL, *d = NULL, *e = NULL;
+        _cleanup_free_ char *u = NULL, *v = NULL, *d = NULL, *e = NULL, *u_escaped = NULL, *v_escaped = NULL, *root_hash_escaped = NULL;
         _cleanup_fclose_ FILE *f = NULL;
         const char *p, *to;
         int r;
@@ -75,6 +76,13 @@ static int create_device(void) {
         if (!v)
                 return log_oom();
 
+        u_escaped = specifier_escape(u);
+        if (!u_escaped)
+                return log_oom();
+        v_escaped = specifier_escape(v);
+        if (!v_escaped)
+                return log_oom();
+
         r = unit_name_from_path(u, ".device", &d);
         if (r < 0)
                 return log_error_errno(r, "Failed to generate unit name: %m");
@@ -82,6 +90,10 @@ static int create_device(void) {
         if (r < 0)
                 return log_error_errno(r, "Failed to generate unit name: %m");
 
+        root_hash_escaped = specifier_escape(arg_root_hash);
+        if (!root_hash_escaped)
+                return log_oom();
+
         f = fopen(p, "wxe");
         if (!f)
                 return log_error_errno(errno, "Failed to create unit file %s: %m", p);
@@ -105,7 +117,7 @@ static int create_device(void) {
                 "ExecStop=" ROOTLIBEXECDIR "/systemd-veritysetup detach root\n",
                 d, e,
                 d, e,
-                u, v, arg_root_hash);
+                u_escaped, v_escaped, root_hash_escaped);
 
         r = fflush_and_check(f);
         if (r < 0)