diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index f70878fb8e1..06bae8decf4 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -14,6 +14,7 @@ #include "dirent-util.h" #include "fd-util.h" #include "fs-util.h" +#include "locale-util.h" #include "log.h" #include "macro.h" #include "missing.h" @@ -663,15 +664,42 @@ int inotify_add_watch_fd(int fd, int what, uint32_t mask) { return r; } -static bool safe_transition(const struct stat *a, const struct stat *b) { +static bool unsafe_transition(const struct stat *a, const struct stat *b) { /* Returns true if the transition from a to b is safe, i.e. that we never transition from unprivileged to * privileged files or directories. Why bother? So that unprivileged code can't symlink to privileged files * making us believe we read something safe even though it isn't safe in the specific context we open it in. */ if (a->st_uid == 0) /* Transitioning from privileged to unprivileged is always fine */ - return true; + return false; - return a->st_uid == b->st_uid; /* Otherwise we need to stay within the same UID */ + return a->st_uid != b->st_uid; /* Otherwise we need to stay within the same UID */ +} + +static int log_unsafe_transition(int a, int b, const char *path, unsigned flags) { + _cleanup_free_ char *n1 = NULL, *n2 = NULL; + + if (!FLAGS_SET(flags, CHASE_WARN)) + return -ENOLINK; + + (void) fd_get_path(a, &n1); + (void) fd_get_path(b, &n2); + + return log_warning_errno(SYNTHETIC_ERRNO(ENOLINK), + "Detected unsafe path transition %s %s %s during canonicalization of %s.", + n1, special_glyph(ARROW), n2, path); +} + +static int log_autofs_mount_point(int fd, const char *path, unsigned flags) { + _cleanup_free_ char *n1 = NULL; + + if (!FLAGS_SET(flags, CHASE_WARN)) + return -EREMOTE; + + (void) fd_get_path(fd, &n1); + + return log_warning_errno(SYNTHETIC_ERRNO(EREMOTE), + "Detected autofs mount point %s during canonicalization of %s.", + n1, path); } int chase_symlinks(const char *path, const char *original_root, unsigned flags, char **ret) { @@ -734,6 +762,14 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags, * path is fully normalized, and == 0 for each normalization step. This may be combined with * CHASE_NONEXISTENT, in which case 1 is returned when a component is not found. * + * 4. With CHASE_SAFE: in this case the path must not contain unsafe transitions, i.e. transitions from + * unprivileged to privileged files or directories. In such cases the return value is -ENOLINK. If + * CHASE_WARN is also set a warning describing the unsafe transition is emitted. + * + * 5. With CHASE_NO_AUTOFS: in this case if an autofs mount point is encountered, the path normalization is + * aborted and -EREMOTE is returned. If CHASE_WARN is also set a warning showing the path of the mount point + * is emitted. + * * */ /* A root directory of "/" or "" is identical to none */ @@ -848,8 +884,8 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags, if (fstat(fd_parent, &st) < 0) return -errno; - if (!safe_transition(&previous_stat, &st)) - return -EPERM; + if (unsafe_transition(&previous_stat, &st)) + return log_unsafe_transition(fd, fd_parent, path, flags); previous_stat = st; } @@ -889,14 +925,14 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags, if (fstat(child, &st) < 0) return -errno; if ((flags & CHASE_SAFE) && - !safe_transition(&previous_stat, &st)) - return -EPERM; + unsafe_transition(&previous_stat, &st)) + return log_unsafe_transition(fd, child, path, flags); previous_stat = st; if ((flags & CHASE_NO_AUTOFS) && fd_is_fs_type(child, AUTOFS_SUPER_MAGIC) > 0) - return -EREMOTE; + return log_autofs_mount_point(child, path, flags); if (S_ISLNK(st.st_mode) && !((flags & CHASE_NOFOLLOW) && isempty(todo))) { char *joined; @@ -928,8 +964,8 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags, if (fstat(fd, &st) < 0) return -errno; - if (!safe_transition(&previous_stat, &st)) - return -EPERM; + if (unsafe_transition(&previous_stat, &st)) + return log_unsafe_transition(child, fd, path, flags); previous_stat = st; } diff --git a/src/basic/fs-util.h b/src/basic/fs-util.h index 955b146a6a4..7ad030be5d0 100644 --- a/src/basic/fs-util.h +++ b/src/basic/fs-util.h @@ -74,6 +74,7 @@ enum { CHASE_TRAIL_SLASH = 1 << 5, /* If set, any trailing slash will be preserved */ CHASE_STEP = 1 << 6, /* If set, just execute a single step of the normalization */ CHASE_NOFOLLOW = 1 << 7, /* Only valid with CHASE_OPEN: when the path's right-most component refers to symlink return O_PATH fd of the symlink, rather than following it. */ + CHASE_WARN = 1 << 8, /* Emit an appropriate warning when an error is encountered */ }; /* How many iterations to execute before returning -ELOOP */ diff --git a/src/core/service.c b/src/core/service.c index 76f1e160697..d631687205e 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -935,8 +935,8 @@ static int service_load_pid_file(Service *s, bool may_warn) { prio = may_warn ? LOG_INFO : LOG_DEBUG; fd = chase_symlinks(s->pid_file, NULL, CHASE_OPEN|CHASE_SAFE, NULL); - if (fd == -EPERM) { - log_unit_full(UNIT(s), LOG_DEBUG, fd, "Permission denied while opening PID file or potentially unsafe symlink chain, will now retry with relaxed checks: %s", s->pid_file); + if (fd == -ENOLINK) { + log_unit_full(UNIT(s), LOG_DEBUG, fd, "Potentially unsafe symlink chain, will now retry with relaxed checks: %s", s->pid_file); questionable_pid_file = true; diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c index 75466a1f1c4..b3a4b1749c2 100644 --- a/src/test/test-fs-util.c +++ b/src/test/test-fs-util.c @@ -248,11 +248,11 @@ static void test_chase_symlinks(void) { assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) >= 0); assert_se(chown(q, 0, 0) >= 0); - assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) == -EPERM); + assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) == -ENOLINK); assert_se(rmdir(q) >= 0); assert_se(symlink("/etc/passwd", q) >= 0); - assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) == -EPERM); + assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) == -ENOLINK); assert_se(chown(p, 0, 0) >= 0); assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) >= 0); diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 9cd317e97b1..08c57a3fb16 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -855,10 +855,8 @@ static int path_open_parent_safe(const char *path) { if (!dn) return log_oom(); - fd = chase_symlinks(dn, NULL, CHASE_OPEN|CHASE_SAFE, NULL); - if (fd == -EPERM) - return log_error_errno(fd, "Unsafe symlinks encountered in %s, refusing.", path); - if (fd < 0) + fd = chase_symlinks(dn, NULL, CHASE_OPEN|CHASE_SAFE|CHASE_WARN, NULL); + if (fd < 0 && fd != -ENOLINK) return log_error_errno(fd, "Failed to validate path %s: %m", path); return fd; @@ -878,10 +876,8 @@ static int path_open_safe(const char *path) { "Failed to open invalid path '%s'.", path); - fd = chase_symlinks(path, NULL, CHASE_OPEN|CHASE_SAFE|CHASE_NOFOLLOW, NULL); - if (fd == -EPERM) - return log_error_errno(fd, "Unsafe symlinks encountered in %s, refusing.", path); - if (fd < 0) + fd = chase_symlinks(path, NULL, CHASE_OPEN|CHASE_SAFE|CHASE_WARN|CHASE_NOFOLLOW, NULL); + if (fd < 0 && fd != -ENOLINK) return log_error_errno(fd, "Failed to validate path %s: %m", path); return fd; @@ -2259,11 +2255,12 @@ static int process_item(Item *i, OperationMask operation) { i->done |= operation; - r = chase_symlinks(i->path, NULL, CHASE_NO_AUTOFS, NULL); + r = chase_symlinks(i->path, NULL, CHASE_NO_AUTOFS|CHASE_WARN, NULL); if (r == -EREMOTE) { - log_debug_errno(r, "Item '%s' is below autofs, skipping.", i->path); + log_notice_errno(r, "Skipping %s", i->path); return 0; - } else if (r < 0) + } + if (r < 0) log_debug_errno(r, "Failed to determine whether '%s' is below autofs, ignoring: %m", i->path); r = FLAGS_SET(operation, OPERATION_CREATE) ? create_item(i) : 0;