From 36c97decbe46a62b6976b8ba7c59e8552f14f935 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 30 Nov 2018 15:13:44 +0100 Subject: [PATCH] fs-util: make chase_symlink() returns -ENOLINK when unsafe transitions are met We previously returned -EPERM but it can be returned for various other reasons too. Let's use -ENOLINK instead as this value shouldn't be used currently. This allows users of CHASE_SAFE to detect without any ambiguities when unsafe transitions are encountered by chase_symlinks(). All current users of CHASE_SAFE that explicitly reacted on -EPERM have been converted to react on -ENOLINK. --- src/basic/fs-util.c | 8 ++++++-- src/core/service.c | 4 ++-- src/test/test-fs-util.c | 4 ++-- src/tmpfiles/tmpfiles.c | 4 ++-- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index e45ad06491..f91d338507 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -649,12 +649,12 @@ 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 -EPERM; + return -ENOLINK; (void) fd_get_path(a, &n1); (void) fd_get_path(b, &n2); - return log_warning_errno(SYNTHETIC_ERRNO(EPERM), + return log_warning_errno(SYNTHETIC_ERRNO(ENOLINK), "Detected unsafe path transition %s %s %s during canonicalization of %s.", n1, special_glyph(ARROW), n2, path); } @@ -719,6 +719,10 @@ 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. + * * */ /* A root directory of "/" or "" is identical to none */ diff --git a/src/core/service.c b/src/core/service.c index 964a7fd057..1fafb33f23 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -934,8 +934,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 d5f652c938..26980d939f 100644 --- a/src/test/test-fs-util.c +++ b/src/test/test-fs-util.c @@ -249,11 +249,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 eeeb1d1850..1f2caf5f73 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -862,7 +862,7 @@ static int path_open_parent_safe(const char *path) { return log_oom(); fd = chase_symlinks(dn, NULL, CHASE_OPEN|CHASE_SAFE, NULL); - if (fd == -EPERM) + if (fd == -ENOLINK) return log_error_errno(fd, "Unsafe symlinks encountered in %s, refusing.", path); if (fd < 0) return log_error_errno(fd, "Failed to validate path %s: %m", path); @@ -885,7 +885,7 @@ static int path_open_safe(const char *path) { path); fd = chase_symlinks(path, NULL, CHASE_OPEN|CHASE_SAFE|CHASE_NOFOLLOW, NULL); - if (fd == -EPERM) + if (fd == -ENOLINK) return log_error_errno(fd, "Unsafe symlinks encountered in %s, refusing.", path); if (fd < 0) return log_error_errno(fd, "Failed to validate path %s: %m", path);