From a2a5291b3f5ab6ed4c92f51d0fd10a03047380d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 29 Jul 2014 22:01:36 -0400 Subject: Reject invalid quoted strings String which ended in an unfinished quote were accepted, potentially with bad memory accesses. Reject anything which ends in a unfished quote, or contains non-whitespace characters right after the closing quote. _FOREACH_WORD now returns the invalid character in *state. But this return value is not checked anywhere yet. Also, make 'word' and 'state' variables const pointers, and rename 'w' to 'word' in various places. Things are easier to read if the same name is used consistently. mbiebl_> am I correct that something like this doesn't work mbiebl_> ExecStart=/usr/bin/encfs --extpass='/bin/systemd-ask-passwd "Unlock EncFS"' mbiebl_> systemd seems to strip of the quotes mbiebl_> systemctl status shows mbiebl_> ExecStart=/usr/bin/encfs --extpass='/bin/systemd-ask-password Unlock EncFS $RootDir $MountPoint mbiebl_> which is pretty weird --- src/core/device.c | 12 ++-- src/core/load-fragment.c | 139 +++++++++++++++++++++++------------------------ src/core/main.c | 13 ++--- 3 files changed, 80 insertions(+), 84 deletions(-) (limited to 'src/core') diff --git a/src/core/device.c b/src/core/device.c index 444286e02..2c41c7b6f 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -220,7 +220,7 @@ static int device_make_description(Unit *u, struct udev_device *dev, const char static int device_add_udev_wants(Unit *u, struct udev_device *dev) { const char *wants; - char *state, *w; + const char *word, *state; size_t l; int r; @@ -234,11 +234,11 @@ static int device_add_udev_wants(Unit *u, struct udev_device *dev) { if (!wants) return 0; - FOREACH_WORD_QUOTED(w, l, wants, state) { + FOREACH_WORD_QUOTED(word, l, wants, state) { _cleanup_free_ char *n = NULL; char e[l+1]; - memcpy(e, w, l); + memcpy(e, word, l); e[l] = 0; n = unit_name_mangle(e, MANGLE_NOGLOB); @@ -393,13 +393,13 @@ static int device_process_new_device(Manager *m, struct udev_device *dev) { * aliases */ alias = udev_device_get_property_value(dev, "SYSTEMD_ALIAS"); if (alias) { - char *state, *w; + const char *word, *state; size_t l; - FOREACH_WORD_QUOTED(w, l, alias, state) { + FOREACH_WORD_QUOTED(word, l, alias, state) { char e[l+1]; - memcpy(e, w, l); + memcpy(e, word, l); e[l] = 0; if (path_is_absolute(e)) diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 6d8431015..b0448e2c4 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -97,18 +97,18 @@ int config_parse_unit_deps(const char* unit, UnitDependency d = ltype; Unit *u = userdata; - char *w, *state; + const char *word, *state; size_t l; assert(filename); assert(lvalue); assert(rvalue); - FOREACH_WORD_QUOTED(w, l, rvalue, state) { + FOREACH_WORD_QUOTED(word, l, rvalue, state) { _cleanup_free_ char *t = NULL, *k = NULL; int r; - t = strndup(w, l); + t = strndup(word, l); if (!t) return log_oom(); @@ -227,7 +227,8 @@ int config_parse_unit_path_strv_printf( void *data, void *userdata) { - char *w, *state, ***x = data; + char ***x = data; + const char *word, *state; Unit *u = userdata; size_t l; int r; @@ -237,11 +238,11 @@ int config_parse_unit_path_strv_printf( assert(rvalue); assert(u); - FOREACH_WORD_QUOTED(w, l, rvalue, state) { + FOREACH_WORD_QUOTED(word, l, rvalue, state) { _cleanup_free_ char *k = NULL; char t[l+1]; - memcpy(t, w, l); + memcpy(t, word, l); t[l] = 0; r = unit_full_printf(u, t, &k); @@ -533,9 +534,8 @@ int config_parse_exec(const char *unit, * overriding of argv[0]. */ for (;;) { int i; - char *w; + const char *word, *state; size_t l; - char *state; bool honour_argv0 = false, ignore = false; path = NULL; @@ -566,8 +566,8 @@ int config_parse_exec(const char *unit, } k = 0; - FOREACH_WORD_QUOTED(w, l, rvalue, state) { - if (strneq(w, ";", MAX(l, 1U))) + FOREACH_WORD_QUOTED(word, l, rvalue, state) { + if (strneq(word, ";", MAX(l, 1U))) break; k++; @@ -578,16 +578,16 @@ int config_parse_exec(const char *unit, return log_oom(); k = 0; - FOREACH_WORD_QUOTED(w, l, rvalue, state) { - if (strneq(w, ";", MAX(l, 1U))) + FOREACH_WORD_QUOTED(word, l, rvalue, state) { + if (strneq(word, ";", MAX(l, 1U))) break; - else if (strneq(w, "\\;", MAX(l, 1U))) - w ++; + else if (strneq(word, "\\;", MAX(l, 1U))) + word ++; - if (honour_argv0 && w == rvalue) { + if (honour_argv0 && word == rvalue) { assert(!path); - path = strndup(w, l); + path = strndup(word, l); if (!path) { r = log_oom(); goto fail; @@ -602,7 +602,7 @@ int config_parse_exec(const char *unit, } else { char *c; - c = n[k++] = cunescape_length(w, l); + c = n[k++] = cunescape_length(word, l); if (!c) { r = log_oom(); goto fail; @@ -854,9 +854,8 @@ int config_parse_exec_cpu_affinity(const char *unit, void *userdata) { ExecContext *c = data; - char *w; + const char *word, *state; size_t l; - char *state; assert(filename); assert(lvalue); @@ -871,12 +870,12 @@ int config_parse_exec_cpu_affinity(const char *unit, return 0; } - FOREACH_WORD_QUOTED(w, l, rvalue, state) { + FOREACH_WORD_QUOTED(word, l, rvalue, state) { _cleanup_free_ char *t = NULL; int r; unsigned cpu; - t = strndup(w, l); + t = strndup(word, l); if (!t) return log_oom(); @@ -945,9 +944,8 @@ int config_parse_exec_secure_bits(const char *unit, void *userdata) { ExecContext *c = data; - char *w; size_t l; - char *state; + const char *word, *state; assert(filename); assert(lvalue); @@ -960,18 +958,18 @@ int config_parse_exec_secure_bits(const char *unit, return 0; } - FOREACH_WORD_QUOTED(w, l, rvalue, state) { - if (first_word(w, "keep-caps")) + FOREACH_WORD_QUOTED(word, l, rvalue, state) { + if (first_word(word, "keep-caps")) c->secure_bits |= 1<secure_bits |= 1<secure_bits |= 1<secure_bits |= 1<secure_bits |= 1<secure_bits |= 1<address_families_whitelist = !invert; } - FOREACH_WORD_QUOTED(w, l, rvalue, state) { + FOREACH_WORD_QUOTED(word, l, rvalue, state) { _cleanup_free_ char *t = NULL; int af; - t = strndup(w, l); + t = strndup(word, l); if (!t) return log_oom(); @@ -2874,7 +2870,8 @@ int config_parse_runtime_directory( void *data, void *userdata) { - char***rt = data, *w, *state; + char***rt = data; + const char *word, *state; size_t l; int r; @@ -2890,10 +2887,10 @@ int config_parse_runtime_directory( return 0; } - FOREACH_WORD_QUOTED(w, l, rvalue, state) { + FOREACH_WORD_QUOTED(word, l, rvalue, state) { _cleanup_free_ char *n; - n = strndup(w, l); + n = strndup(word, l); if (!n) return log_oom(); @@ -2924,9 +2921,8 @@ int config_parse_set_status( void *data, void *userdata) { - char *w; size_t l; - char *state; + const char *word, *state; int r; ExitStatusSet *status_set = data; @@ -2941,11 +2937,11 @@ int config_parse_set_status( return 0; } - FOREACH_WORD(w, l, rvalue, state) { + FOREACH_WORD(word, l, rvalue, state) { _cleanup_free_ char *temp; int val; - temp = strndup(w, l); + temp = strndup(word, l); if (!temp) return log_oom(); @@ -2960,11 +2956,11 @@ int config_parse_set_status( r = set_put(status_set->signal, INT_TO_PTR(val)); if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, -r, "Unable to store: %s", w); + log_syntax(unit, LOG_ERR, filename, line, -r, "Unable to store: %s", word); return r; } } else { - log_syntax(unit, LOG_ERR, filename, line, -val, "Failed to parse value, ignoring: %s", w); + log_syntax(unit, LOG_ERR, filename, line, -val, "Failed to parse value, ignoring: %s", word); return 0; } } else { @@ -2977,7 +2973,7 @@ int config_parse_set_status( r = set_put(status_set->status, INT_TO_PTR(val)); if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, -r, "Unable to store: %s", w); + log_syntax(unit, LOG_ERR, filename, line, -r, "Unable to store: %s", word); return r; } } @@ -2999,7 +2995,8 @@ int config_parse_namespace_path_strv( void *data, void *userdata) { - char*** sv = data, *w, *state; + char*** sv = data; + const char *word, *state; size_t l; int r; @@ -3015,11 +3012,11 @@ int config_parse_namespace_path_strv( return 0; } - FOREACH_WORD_QUOTED(w, l, rvalue, state) { + FOREACH_WORD_QUOTED(word, l, rvalue, state) { _cleanup_free_ char *n; int offset; - n = strndup(w, l); + n = strndup(word, l); if (!n) return log_oom(); diff --git a/src/core/main.c b/src/core/main.c index 2741989c4..fad15c7c3 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -455,9 +455,8 @@ static int config_parse_cpu_affinity2( void *data, void *userdata) { - char *w; + const char *word, *state; size_t l; - char *state; cpu_set_t *c = NULL; unsigned ncpus = 0; @@ -465,12 +464,12 @@ static int config_parse_cpu_affinity2( assert(lvalue); assert(rvalue); - FOREACH_WORD_QUOTED(w, l, rvalue, state) { + FOREACH_WORD_QUOTED(word, l, rvalue, state) { char *t; int r; unsigned cpu; - if (!(t = strndup(w, l))) + if (!(t = strndup(word, l))) return log_oom(); r = safe_atou(t, &cpu); @@ -559,7 +558,7 @@ static int config_parse_join_controllers(const char *unit, void *userdata) { unsigned n = 0; - char *state, *w; + const char *word, *state; size_t length; assert(filename); @@ -568,10 +567,10 @@ static int config_parse_join_controllers(const char *unit, free_join_controllers(); - FOREACH_WORD_QUOTED(w, length, rvalue, state) { + FOREACH_WORD_QUOTED(word, length, rvalue, state) { char *s, **l; - s = strndup(w, length); + s = strndup(word, length); if (!s) return log_oom(); -- cgit v1.2.3