From: Lennart Poettering Date: Mon, 15 Apr 2013 12:05:00 +0000 (+0200) Subject: bus: handle env vars safely X-Git-Tag: v202~92 X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?p=elogind.git;a=commitdiff_plain;h=6c03089c32c251d823173bda4d809a9e643219f0 bus: handle env vars safely Make sure that our library is safe for usage in SUID programs when it comes to env var handling --- diff --git a/src/libsystemd-bus/sd-bus.c b/src/libsystemd-bus/sd-bus.c index f2dd81235..c7511c32d 100644 --- a/src/libsystemd-bus/sd-bus.c +++ b/src/libsystemd-bus/sd-bus.c @@ -31,6 +31,7 @@ #include "macro.h" #include "strv.h" #include "set.h" +#include "missing.h" #include "sd-bus.h" #include "bus-internal.h" @@ -841,7 +842,7 @@ int sd_bus_open_system(sd_bus **ret) { if (r < 0) return r; - e = getenv("DBUS_SYSTEM_BUS_ADDRESS"); + e = secure_getenv("DBUS_SYSTEM_BUS_ADDRESS"); if (e) { r = sd_bus_set_address(b, e); if (r < 0) @@ -879,13 +880,13 @@ int sd_bus_open_user(sd_bus **ret) { if (r < 0) return r; - e = getenv("DBUS_SESSION_BUS_ADDRESS"); + e = secure_getenv("DBUS_SESSION_BUS_ADDRESS"); if (e) { r = sd_bus_set_address(b, e); if (r < 0) goto fail; } else { - e = getenv("XDG_RUNTIME_DIR"); + e = secure_getenv("XDG_RUNTIME_DIR"); if (!e) { r = -ENOENT; goto fail; diff --git a/src/shared/cgroup-util.c b/src/shared/cgroup-util.c index c17e1d4d1..075260783 100644 --- a/src/shared/cgroup-util.c +++ b/src/shared/cgroup-util.c @@ -1148,8 +1148,6 @@ int cg_get_user_path(char **path) { char **cg_shorten_controllers(char **controllers) { char **f, **t; - controllers = strv_uniq(controllers); - if (!controllers) return controllers; @@ -1175,11 +1173,11 @@ char **cg_shorten_controllers(char **controllers) { } *t = NULL; - return controllers; + return strv_uniq(controllers); } int cg_pid_get_cgroup(pid_t pid, char **root, char **cgroup) { - char *cg_process, *cg_init, *p; + char *cg_process, *cg_init, *p, *q; int r; assert(pid >= 0); @@ -1202,11 +1200,8 @@ int cg_pid_get_cgroup(pid_t pid, char **root, char **cgroup) { else if (streq(cg_init, "/")) cg_init[0] = 0; - if (startswith(cg_process, cg_init)) - p = cg_process + strlen(cg_init); - else - p = cg_process; - + q = startswith(cg_process, cg_init); + p = q ? q : cg_process; free(cg_init); if (cgroup) { @@ -1230,84 +1225,126 @@ int cg_pid_get_cgroup(pid_t pid, char **root, char **cgroup) { return 0; } -static int instance_unit_from_cgroup(char *cgroup){ - char *at; +/* non-static only for testing purposes */ +int cg_cgroup_to_unit(const char *cgroup, char **unit){ + char *p, *e, *c, *s, *k; assert(cgroup); + assert(unit); - at = strstr(cgroup, "@."); - if (at) { - /* This is a templated service */ + e = strchrnul(cgroup, '/'); + c = strndupa(cgroup, e - cgroup); - char *i; - char _cleanup_free_ *i2 = NULL, *s = NULL; + /* Could this be a valid unit name? */ + if (!unit_name_is_valid(c, true)) + return -EINVAL; - i = strchr(at, '/'); - if (!i || !i[1]) /* disallow empty instances */ + if (!unit_name_is_template(c)) + s = strdup(c); + else { + if (*e != '/') return -EINVAL; - s = strndup(at + 1, i - at - 1); - i2 = strdup(i + 1); - if (!s || !i2) - return -ENOMEM; + e += strspn(e, "/"); + p = strchrnul(e, '/'); - strcpy(at + 1, i2); - strcat(at + 1, s); + /* Don't allow empty instance strings */ + if (p == e) + return -EINVAL; + + k = strndupa(e, p - e); + + s = unit_name_replace_instance(c, k); } + if (!s) + return -ENOMEM; + + *unit = s; return 0; } -/* non-static only for testing purposes */ -int cgroup_to_unit(char *cgroup, char **unit){ +int cg_path_get_unit(const char *path, char **unit) { + const char *e; + + assert(path); + assert(unit); + + e = path_startswith(path, "/system/"); + if (!e) + return -ENOENT; + + return cg_cgroup_to_unit(e, unit); +} + +int cg_pid_get_unit(pid_t pid, char **unit) { + char _cleanup_free_ *cgroup = NULL; int r; - char *p; - assert(cgroup); assert(unit); - r = instance_unit_from_cgroup(cgroup); + r = cg_pid_get_cgroup(pid, NULL, &cgroup); if (r < 0) return r; - p = strrchr(cgroup, '/'); - assert(p); + return cg_path_get_unit(cgroup, unit); +} - r = unit_name_is_valid(p + 1, true); - if (!r) - return -EINVAL; +static const char *skip_label(const char *e) { + assert(e); - *unit = strdup(p + 1); - if (!*unit) - return -ENOMEM; + e += strspn(e, "/"); + e = strchr(e, '/'); + if (!e) + return NULL; - return 0; + e += strspn(e, "/"); + return e; } -static int cg_pid_get(const char *prefix, pid_t pid, char **unit) { - int r; - char _cleanup_free_ *cgroup = NULL; +int cg_path_get_user_unit(const char *path, char **unit) { + const char *e; - assert(pid >= 0); + assert(path); assert(unit); - r = cg_pid_get_cgroup(pid, NULL, &cgroup); - if (r < 0) - return r; + /* We always have to parse the path from the beginning as unit + * cgroups might have arbitrary child cgroups and we shouldn't get + * confused by those */ - if (!startswith(cgroup, prefix)) + e = path_startswith(path, "/user/"); + if (!e) return -ENOENT; - r = cgroup_to_unit(cgroup, unit); - return r; -} + /* Skip the user name */ + e = skip_label(e); + if (!e) + return -ENOENT; -int cg_pid_get_unit(pid_t pid, char **unit) { - return cg_pid_get("/system/", pid, unit); + /* Skip the session ID */ + e = skip_label(e); + if (!e) + return -ENOENT; + + /* Skip the systemd cgroup */ + e = skip_label(e); + if (!e) + return -ENOENT; + + return cg_cgroup_to_unit(e, unit); } int cg_pid_get_user_unit(pid_t pid, char **unit) { - return cg_pid_get("/user/", pid, unit); + char _cleanup_free_ *cgroup = NULL; + int r; + + assert(unit); + + r = cg_pid_get_cgroup(pid, NULL, &cgroup); + if (r < 0) + return r; + + return cg_path_get_user_unit(cgroup, unit); } int cg_controller_from_attr(const char *attr, char **controller) { diff --git a/src/shared/cgroup-util.h b/src/shared/cgroup-util.h index 06c6bfb2e..123f72c69 100644 --- a/src/shared/cgroup-util.h +++ b/src/shared/cgroup-util.h @@ -69,11 +69,15 @@ int cg_is_empty_by_spec(const char *spec, bool ignore_self); int cg_is_empty_recursive(const char *controller, const char *path, bool ignore_self); int cg_get_user_path(char **path); + +int cg_path_get_unit(const char *path, char **unit); +int cg_path_get_user_unit(const char *path, char **unit); + int cg_pid_get_cgroup(pid_t pid, char **root, char **cgroup); int cg_pid_get_unit(pid_t pid, char **unit); int cg_pid_get_user_unit(pid_t pid, char **unit); -int cgroup_to_unit(char *cgroup, char **unit); +int cg_cgroup_to_unit(const char *cgroup, char **unit); char **cg_shorten_controllers(char **controllers); diff --git a/src/test/test-cgroup-util.c b/src/test/test-cgroup-util.c index b30bf23a8..8e24d1cec 100644 --- a/src/test/test-cgroup-util.c +++ b/src/test/test-cgroup-util.c @@ -1,27 +1,79 @@ +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/ + +/*** + This file is part of systemd. + + Copyright 2013 Zbigniew Jędrzejewski-Szmek + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2.1 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with systemd; If not, see . +***/ + #include #include "util.h" #include "cgroup-util.h" -#define check_c_t_u(path, code, result) \ -{ \ - char a[] = path; \ - char *unit = NULL; \ - assert_se(cgroup_to_unit(a, &unit) == code); \ - assert(code < 0 || streq(unit, result)); \ -} +static void check_c_t_u(const char *path, int code, const char *result) { + _cleanup_free_ char *unit = NULL; + assert_se(cg_cgroup_to_unit(path, &unit) == code); + assert_se(streq_ptr(unit, result)); +} static void test_cgroup_to_unit(void) { - check_c_t_u("/system/getty@.service/tty2", 0, "getty@tty2.service"); - check_c_t_u("/system/getty@.service/", -EINVAL, "getty@tty2.service"); - check_c_t_u("/system/getty@.service", -EINVAL, "getty@tty2.service"); - check_c_t_u("/system/getty.service", 0, "getty.service"); - check_c_t_u("/system/getty", -EINVAL, "getty.service"); + check_c_t_u("getty@.service/tty2", 0, "getty@tty2.service"); + check_c_t_u("getty@.service/tty2/xxx", 0, "getty@tty2.service"); + check_c_t_u("getty@.service/", -EINVAL, NULL); + check_c_t_u("getty@.service", -EINVAL, NULL); + check_c_t_u("getty.service", 0, "getty.service"); + check_c_t_u("getty", -EINVAL, NULL); +} + +static void check_p_g_u(const char *path, int code, const char *result) { + _cleanup_free_ char *unit = NULL; + + assert_se(cg_path_get_unit(path, &unit) == code); + assert_se(streq_ptr(unit, result)); +} + +static void check_p_g_u_u(const char *path, int code, const char *result) { + _cleanup_free_ char *unit = NULL; + + assert_se(cg_path_get_user_unit(path, &unit) == code); + assert_se(streq_ptr(unit, result)); +} + +static void test_path_get_unit(void) { + check_p_g_u("/system/foobar.service/sdfdsaf", 0, "foobar.service"); + check_p_g_u("/system/getty@.service/tty5", 0, "getty@tty5.service"); + check_p_g_u("/system/getty@.service/tty5/aaa/bbb", 0, "getty@tty5.service"); + check_p_g_u("/system/getty@.service/tty5/", 0, "getty@tty5.service"); + check_p_g_u("/system/getty@tty6.service/tty5", 0, "getty@tty6.service"); + check_p_g_u("sadfdsafsda", -ENOENT, NULL); + check_p_g_u("/system/getty####@tty6.service/tty5", -EINVAL, NULL); + + check_p_g_u_u("/user/lennart/2/systemd-21548/foobar.service", 0, "foobar.service"); + check_p_g_u_u("/user/lennart/2/systemd-21548/foobar.service/waldo", 0, "foobar.service"); + check_p_g_u_u("/user/lennart/2/systemd-21548/foobar.service/waldo/uuuux", 0, "foobar.service"); + check_p_g_u_u("/user/lennart/2/systemd-21548/waldo/waldo/uuuux", -EINVAL, NULL); + check_p_g_u_u("/user/lennart/2/foobar.service", -ENOENT, NULL); + check_p_g_u_u("/user/lennart/2/systemd-21548/foobar@.service/pie/pa/po", 0, "foobar@pie.service"); } int main(void) { test_cgroup_to_unit(); + test_path_get_unit(); return 0; }