summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLennart Poettering <lennart@poettering.net>2013-01-19 00:59:19 +0100
committerLennart Poettering <lennart@poettering.net>2013-01-19 01:02:30 +0100
commite884315e3d28df0d5f4e7d4590730e9760b8f447 (patch)
treeea794f1ccbc81cd45982d36ee1996ed28e265c7a
parentadf36dd0aec564a00a7445e328c3e27f44938629 (diff)
cgroup: additional validity checks for cgroup attribute names
-rw-r--r--src/core/unit.c19
-rw-r--r--src/shared/cgroup-util.c64
-rw-r--r--src/shared/cgroup-util.h2
-rw-r--r--src/shared/util.c37
-rw-r--r--src/shared/util.h1
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,