From 99be45a46fc9b45046a0f7b83a9f3e321f29d2da Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 27 Oct 2017 16:28:15 +0200 Subject: [PATCH] =?UTF-8?q?fs-util:=20rename=20path=5Fis=5Fsafe()=20?= =?UTF-8?q?=E2=86=92=20path=5Fis=5Fnormalized()?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Already, path_is_safe() refused paths container the "." dir. Doing that isn't strictly necessary to be "safe" by most definitions of the word. But it is necessary in order to consider a path "normalized". Hence, "path_is_safe()" is slightly misleading a name, but "path_is_normalize()" is more descriptive, hence let's rename things accordingly. No functional changes. --- src/basic/cgroup-util.c | 4 ++-- src/basic/fs-util.c | 4 ++-- src/basic/path-util.c | 3 +-- src/basic/path-util.h | 2 +- src/basic/unit-name.c | 4 ++-- src/basic/user-util.c | 2 +- src/core/dbus-execute.c | 4 ++-- src/core/load-fragment.c | 6 +++--- src/core/unit.c | 2 +- src/machine/machine-dbus.c | 4 ++-- src/mount/mount-tool.c | 12 ++++++------ 11 files changed, 23 insertions(+), 24 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index b52104768fc..63aee015d02 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -1278,7 +1278,7 @@ int cg_split_spec(const char *spec, char **controller, char **path) { assert(spec); if (*spec == '/') { - if (!path_is_safe(spec)) + if (!path_is_normalized(spec)) return -EINVAL; if (path) { @@ -1331,7 +1331,7 @@ int cg_split_spec(const char *spec, char **controller, char **path) { return -ENOMEM; } - if (!path_is_safe(u) || + if (!path_is_normalized(u) || !path_is_absolute(u)) { free(t); free(u); diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 946f779a329..f3cb5d207ab 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -509,7 +509,7 @@ static int getenv_tmp_dir(const char **ret_path) { r = -ENOTDIR; goto next; } - if (!path_is_safe(e)) { + if (!path_is_normalized(e)) { r = -EPERM; goto next; } @@ -707,7 +707,7 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags, if (errno == ENOENT && (flags & CHASE_NONEXISTENT) && - (isempty(todo) || path_is_safe(todo))) { + (isempty(todo) || path_is_normalized(todo))) { /* If CHASE_NONEXISTENT is set, and the path does not exist, then that's OK, return * what we got so far. But don't allow this if the remaining path contains "../ or "./" diff --git a/src/basic/path-util.c b/src/basic/path-util.c index 6c06bd2acb5..9fac7ff6ec0 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -721,7 +721,7 @@ bool filename_is_valid(const char *p) { return true; } -bool path_is_safe(const char *p) { +bool path_is_normalized(const char *p) { if (isempty(p)) return false; @@ -735,7 +735,6 @@ bool path_is_safe(const char *p) { if (strlen(p)+1 > PATH_MAX) return false; - /* The following two checks are not really dangerous, but hey, they still are confusing */ if (startswith(p, "./") || endswith(p, "/.") || strstr(p, "/./")) return false; diff --git a/src/basic/path-util.h b/src/basic/path-util.h index 546246595c2..4b1410a21bc 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -131,7 +131,7 @@ int parse_path_argument_and_warn(const char *path, bool suppress_root, char **ar char* dirname_malloc(const char *path); bool filename_is_valid(const char *p) _pure_; -bool path_is_safe(const char *p) _pure_; +bool path_is_normalized(const char *p) _pure_; char *file_in_same_dir(const char *path, const char *filename); diff --git a/src/basic/unit-name.c b/src/basic/unit-name.c index 27ce432197b..d37b75b72f4 100644 --- a/src/basic/unit-name.c +++ b/src/basic/unit-name.c @@ -382,7 +382,7 @@ int unit_name_path_escape(const char *f, char **ret) { if (STR_IN_SET(p, "/", "")) s = strdup("-"); else { - if (!path_is_safe(p)) + if (!path_is_normalized(p)) return -EINVAL; /* Truncate trailing slashes */ @@ -432,7 +432,7 @@ int unit_name_path_unescape(const char *f, char **ret) { if (!s) return -ENOMEM; - if (!path_is_safe(s)) { + if (!path_is_normalized(s)) { free(s); return -EINVAL; } diff --git a/src/basic/user-util.c b/src/basic/user-util.c index a691a0d3fc4..f1a0958617f 100644 --- a/src/basic/user-util.c +++ b/src/basic/user-util.c @@ -605,7 +605,7 @@ bool valid_home(const char *p) { if (!path_is_absolute(p)) return false; - if (!path_is_safe(p)) + if (!path_is_normalized(p)) return false; /* Colons are used as field separators, and hence not OK */ diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 47446307b7e..2889bf96ba5 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -1862,7 +1862,7 @@ int bus_exec_context_set_transient_property( if (!path_is_absolute(s)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Path %s is not absolute", s); - if (!path_is_safe(s)) + if (!path_is_normalized(s)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Path %s is not normalized", s); if (mode != UNIT_CHECK) { @@ -2462,7 +2462,7 @@ int bus_exec_context_set_transient_property( return r; STRV_FOREACH(p, l) { - if (!path_is_safe(*p) || path_is_absolute(*p)) + if (!path_is_normalized(*p) || path_is_absolute(*p)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "%s= path is not valid: %s", name, *p); } diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index b4405b5e212..2784f6ba807 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -886,7 +886,7 @@ int config_parse_exec_input( return -EINVAL; } - if (!path_is_safe(resolved)) { + if (!path_is_normalized(resolved)) { log_syntax(unit, LOG_ERR, filename, line, 0, "file: requires a normalized path name: %s", resolved); return -EINVAL; } @@ -1077,7 +1077,7 @@ int config_parse_exec_output( return -EINVAL; } - if (!path_is_safe(resolved)) { + if (!path_is_normalized(resolved)) { log_syntax(unit, LOG_ERR, filename, line, 0, "file: requires a normalized path name, ignoring: %s", resolved); return -EINVAL; } @@ -4050,7 +4050,7 @@ int config_parse_exec_directories( continue; } - if (!path_is_safe(k) || path_is_absolute(k)) { + if (!path_is_normalized(k) || path_is_absolute(k)) { log_syntax(unit, LOG_ERR, filename, line, 0, "%s= path is not valid, ignoring assignment: %s", lvalue, rvalue); continue; diff --git a/src/core/unit.c b/src/core/unit.c index 25cdc045061..e6f2e23dee7 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -4460,7 +4460,7 @@ int unit_require_mounts_for(Unit *u, const char *path, UnitDependencyMask mask) path_kill_slashes(p); - if (!path_is_safe(p)) { + if (!path_is_normalized(p)) { free(p); return -EPERM; } diff --git a/src/machine/machine-dbus.c b/src/machine/machine-dbus.c index 3cb90be67d7..34a3778ad74 100644 --- a/src/machine/machine-dbus.c +++ b/src/machine/machine-dbus.c @@ -854,12 +854,12 @@ int bus_machine_method_bind_mount(sd_bus_message *message, void *userdata, sd_bu if (r < 0) return r; - if (!path_is_absolute(src) || !path_is_safe(src)) + if (!path_is_absolute(src) || !path_is_normalized(src)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Source path must be absolute and not contain ../."); if (isempty(dest)) dest = src; - else if (!path_is_absolute(dest) || !path_is_safe(dest)) + else if (!path_is_absolute(dest) || !path_is_normalized(dest)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Destination path must be absolute and not contain ../."); r = bus_verify_polkit_async( diff --git a/src/mount/mount-tool.c b/src/mount/mount-tool.c index 7bd40613c35..f6e845e04aa 100644 --- a/src/mount/mount-tool.c +++ b/src/mount/mount-tool.c @@ -886,8 +886,8 @@ static int stop_mounts( return -EINVAL; } - if (!path_is_safe(where)) { - log_error("Path contains unsafe components: %s", where); + if (!path_is_normalized(where)) { + log_error("Path contains non-normalized components: %s", where); return -EINVAL; } @@ -1568,8 +1568,8 @@ int main(int argc, char* argv[]) { goto finish; } - if (!path_is_safe(arg_mount_what)) { - log_error("Path contains unsafe components: %s", arg_mount_what); + if (!path_is_normalized(arg_mount_what)) { + log_error("Path contains non-normalized components: %s", arg_mount_what); r = -EINVAL; goto finish; } @@ -1592,8 +1592,8 @@ int main(int argc, char* argv[]) { goto finish; } - if (!path_is_safe(arg_mount_where)) { - log_error("Path contains unsafe components: %s", arg_mount_where); + if (!path_is_normalized(arg_mount_where)) { + log_error("Path contains non-normalized components: %s", arg_mount_where); r = -EINVAL; goto finish; }