diff options
| -rw-r--r-- | src/core/unit.c | 19 | ||||
| -rw-r--r-- | src/shared/cgroup-util.c | 64 | ||||
| -rw-r--r-- | src/shared/cgroup-util.h | 2 | ||||
| -rw-r--r-- | src/shared/util.c | 37 | ||||
| -rw-r--r-- | src/shared/util.h | 1 |
5 files changed, 89 insertions, 34 deletions
diff --git a/src/core/unit.c b/src/core/unit.c index 83359e126b..6cf02365e9 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -2156,26 +2156,27 @@ int unit_add_cgroup_attribute( _cleanup_free_ char *c = NULL; CGroupAttribute *a; + int r; assert(u); assert(name); assert(value); if (!controller) { - const char *dot; - - dot = strchr(name, '.'); - if (!dot) + r = cg_controller_from_attr(name, &c); + if (r < 0) return -EINVAL; - c = strndup(name, dot - name); - if (!c) - return -ENOMEM; - controller = c; + } else { + if (!filename_is_safe(name)) + return -EINVAL; + + if (!filename_is_safe(controller)) + return -EINVAL; } - if (streq(controller, SYSTEMD_CGROUP_CONTROLLER)) + if (!controller || streq(controller, SYSTEMD_CGROUP_CONTROLLER)) return -EINVAL; a = cgroup_attribute_find_list(u->cgroup_attributes, controller, name); diff --git a/src/shared/cgroup-util.c b/src/shared/cgroup-util.c index f0d0d4855b..acace52bc8 100644 --- a/src/shared/cgroup-util.c +++ b/src/shared/cgroup-util.c @@ -990,6 +990,8 @@ int cg_split_spec(const char *spec, char **controller, char **path) { assert(spec); if (*spec == '/') { + if (!path_is_safe(spec)) + return -EINVAL; if (path) { t = strdup(spec); @@ -1007,7 +1009,7 @@ int cg_split_spec(const char *spec, char **controller, char **path) { e = strchr(spec, ':'); if (!e) { - if (strchr(spec, '/') || spec[0] == 0) + if (!filename_is_safe(spec)) return -EINVAL; if (controller) { @@ -1024,29 +1026,34 @@ int cg_split_spec(const char *spec, char **controller, char **path) { return 0; } - if (e[1] != '/' || e == spec || memchr(spec, '/', e-spec)) + t = strndup(spec, e-spec); + if (!t) + return -ENOMEM; + if (!filename_is_safe(t)) { + free(t); return -EINVAL; - - if (controller) { - t = strndup(spec, e-spec); - if (!t) - return -ENOMEM; - } - if (path) { - u = strdup(e+1); - if (!u) { - free(t); - return -ENOMEM; - } + u = strdup(e+1); + if (!u) { + free(t); + return -ENOMEM; + } + if (!path_is_safe(u)) { + free(t); + free(u); + return -EINVAL; } if (controller) *controller = t; + else + free(t); if (path) *path = u; + else + free(u); return 0; } @@ -1290,3 +1297,32 @@ int cg_pid_get_unit(pid_t pid, char **unit) { int cg_pid_get_user_unit(pid_t pid, char **unit) { return cg_pid_get("/user/", pid, unit); } + +int cg_controller_from_attr(const char *attr, char **controller) { + const char *dot; + char *c; + + assert(attr); + assert(controller); + + if (!filename_is_safe(attr)) + return -EINVAL; + + dot = strchr(attr, '.'); + if (!dot) { + *controller = NULL; + return 0; + } + + c = strndup(attr, dot - attr); + if (!c) + return -ENOMEM; + + if (!filename_is_safe(c)) { + free(c); + return -EINVAL; + } + + *controller = c; + return 1; +} diff --git a/src/shared/cgroup-util.h b/src/shared/cgroup-util.h index 920cf631e5..06c6bfb2e3 100644 --- a/src/shared/cgroup-util.h +++ b/src/shared/cgroup-util.h @@ -76,3 +76,5 @@ int cg_pid_get_user_unit(pid_t pid, char **unit); int cgroup_to_unit(char *cgroup, char **unit); char **cg_shorten_controllers(char **controllers); + +int cg_controller_from_attr(const char *attr, char **controller); diff --git a/src/shared/util.c b/src/shared/util.c index 37e383f2ef..1aaebf0612 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -561,9 +561,9 @@ int fchmod_umask(int fd, mode_t m) { } int write_one_line_file_atomic(const char *fn, const char *line) { - FILE *f; + _cleanup_fclose_ FILE *f = NULL; + _cleanup_free_ char *p = NULL; int r; - char *p; assert(fn); assert(line); @@ -585,12 +585,9 @@ int write_one_line_file_atomic(const char *fn, const char *line) { fflush(f); - if (ferror(f)) { - if (errno != 0) - r = -errno; - else - r = -EIO; - } else { + if (ferror(f)) + r = errno ? -errno : -EIO; + else { if (rename(p, fn) < 0) r = -errno; else @@ -601,9 +598,6 @@ finish: if (r < 0) unlink(p); - fclose(f); - free(p); - return r; } @@ -5613,6 +5607,27 @@ bool string_is_safe(const char *p) { return true; } +bool path_is_safe(const char *p) { + + if (isempty(p)) + return false; + + if (streq(p, "..") || startswith(p, "../") || endswith(p, "/..") || strstr(p, "/../")) + return false; + + if (strlen(p) > PATH_MAX) + return false; + + /* The following two checks are not really dangerous, but hey, they still are confusing */ + if (streq(p, ".") || startswith(p, "./") || endswith(p, "/.") || strstr(p, "/./")) + return false; + + if (strstr(p, "//")) + return false; + + return true; +} + /* hey glibc, APIs with callbacks without a user pointer are so useless */ void *xbsearch_r(const void *key, const void *base, size_t nmemb, size_t size, int (*compar) (const void *, const void *, void *), void *arg) { diff --git a/src/shared/util.h b/src/shared/util.h index cdaff45772..d260385991 100644 --- a/src/shared/util.h +++ b/src/shared/util.h @@ -543,6 +543,7 @@ _malloc_ static inline void *memdup_multiply(const void *p, size_t a, size_t b) } bool filename_is_safe(const char *p); +bool path_is_safe(const char *p); bool string_is_safe(const char *p); void *xbsearch_r(const void *key, const void *base, size_t nmemb, size_t size, |