From: Lennart Poettering Date: Tue, 21 Nov 2017 19:09:31 +0000 (+0100) Subject: generators: be more careful when writing unit settings that support specifier expansion X-Git-Tag: v236~94^2~24 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=98bad05e75cad988d5b2c2cc928f7cc2605cbc2d;p=thirdparty%2Fsystemd.git 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 d0be412625e..e2dc96bdb7e 100644 --- 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 243c1160b38..6d6895a2163 100644 --- 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 7d014db69da..9e8b956d5c3 100644 --- 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 a9670213d4f..462457e2fe5 100644 --- 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 f59c277e13e..51306b56257 100644 --- 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 ef43aed42fb..5919b1380e7 100644 --- 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)