fs-util: add new CHASE_SAFE flag to chase_symlinks()

When the flag is specified we won't transition to a privilege-owned
file or directory from an unprivileged-owned one. This is useful when
privileged code wants to load data from a file unprivileged users have
write access to, and validates the ownership, but want's to make sure
that no symlink games are played to read a root-owned system file
belonging to a different context.

diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c
index a03cbfe35712d166732cf19d79993397a63c79a5..1541635352930ddbbc3e321f41fd7ca9cfc6bdc5 100644 (file)
--- a/src/basic/fs-util.c
+++ b/src/basic/fs-util.c
@@ -39,7 +39,6 @@
 #include "mkdir.h"
 #include "parse-util.h"
 #include "path-util.h"
-//#include "process-util.h"
 #include "stat-util.h"
 #include "stdio-util.h"
 #include "string-util.h"
@@ -626,10 +625,22 @@ int inotify_add_watch_fd(int fd, int what, uint32_t mask) {
 }
 #endif // 0
 
+static bool safe_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 a->st_uid == b->st_uid; /* Otherwise we need to stay within the same UID */
+}
+
 int chase_symlinks(const char *path, const char *original_root, unsigned flags, char **ret) {
         _cleanup_free_ char *buffer = NULL, *done = NULL, *root = NULL;
         _cleanup_close_ int fd = -1;
         unsigned max_follow = 32; /* how many symlinks to follow before giving up and returning ELOOP */
+        struct stat previous_stat;
         bool exists = true;
         char *todo;
         int r;
@@ -673,6 +684,11 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
         if (fd < 0)
                 return -errno;
 
+        if (flags & CHASE_SAFE) {
+                if (fstat(fd, &previous_stat) < 0)
+                        return -errno;
+        }
+
         todo = buffer;
         for (;;) {
                 _cleanup_free_ char *first = NULL;
@@ -734,6 +750,16 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
                         if (fd_parent < 0)
                                 return -errno;
 
+                        if (flags & CHASE_SAFE) {
+                                if (fstat(fd_parent, &st) < 0)
+                                        return -errno;
+
+                                if (!safe_transition(&previous_stat, &st))
+                                        return -EPERM;
+
+                                previous_stat = st;
+                        }
+
                         safe_close(fd);
                         fd = fd_parent;
 
@@ -768,6 +794,12 @@ 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;
+
+                previous_stat = st;
+
                 if ((flags & CHASE_NO_AUTOFS) &&
                     fd_is_fs_type(child, AUTOFS_SUPER_MAGIC) > 0)
                         return -EREMOTE;
@@ -800,6 +832,16 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
 
                                 free(done);
 
+                                if (flags & CHASE_SAFE) {
+                                        if (fstat(fd, &st) < 0)
+                                                return -errno;
+
+                                        if (!safe_transition(&previous_stat, &st))
+                                                return -EPERM;
+
+                                        previous_stat = st;
+                                }
+
                                 /* Note that we do not revalidate the root, we take it as is. */
                                 if (isempty(root))
                                         done = NULL;

diff --git a/src/basic/fs-util.h b/src/basic/fs-util.h
index 8b03055a39c5f6e33e56178f66a1e373f3664e49..13008fdda2b018db3ad0d12f16b0b337a2941077 100644 (file)
--- a/src/basic/fs-util.h
+++ b/src/basic/fs-util.h
@@ -95,9 +95,10 @@ int inotify_add_watch_fd(int fd, int what, uint32_t mask);
 
 #endif // 0
 enum {
-        CHASE_PREFIX_ROOT = 1,   /* If set, the specified path will be prefixed by the specified root before beginning the iteration */
-        CHASE_NONEXISTENT = 2,   /* If set, it's OK if the path doesn't actually exist. */
-        CHASE_NO_AUTOFS = 4,     /* If set, return -EREMOTE if autofs mount point found */
+        CHASE_PREFIX_ROOT = 1U << 0,   /* If set, the specified path will be prefixed by the specified root before beginning the iteration */
+        CHASE_NONEXISTENT = 1U << 1,   /* If set, it's OK if the path doesn't actually exist. */
+        CHASE_NO_AUTOFS   = 1U << 2,   /* If set, return -EREMOTE if autofs mount point found */
+        CHASE_SAFE        = 1U << 3,   /* If set, return EPERM if we ever traverse from unprivileged to privileged files or directories */
 };
 
 int chase_symlinks(const char *path_with_prefix, const char *root, unsigned flags, char **ret);

diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c
index f2f571ce2bf7db588f1591f93e161073dbb0fd11..1c453e11a44011b32743529351a12d3ccb726dca 100644 (file)
--- a/src/test/test-fs-util.c
+++ b/src/test/test-fs-util.c
@@ -30,6 +30,7 @@
 #include "rm-rf.h"
 #include "string-util.h"
 #include "strv.h"
+//#include "user-util.h"
 #include "util.h"
 
 static void test_chase_symlinks(void) {
@@ -235,6 +236,32 @@ static void test_chase_symlinks(void) {
         r = chase_symlinks(p, NULL, 0, &result);
         assert_se(r == -ENOENT);
 
+        if (geteuid() == 0) {
+                p = strjoina(temp, "/priv1");
+                assert_se(mkdir(p, 0755) >= 0);
+
+                q = strjoina(p, "/priv2");
+                assert_se(mkdir(q, 0755) >= 0);
+
+                assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) >= 0);
+
+                assert_se(chown(q, UID_NOBODY, GID_NOBODY) >= 0);
+                assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) >= 0);
+
+                assert_se(chown(p, UID_NOBODY, GID_NOBODY) >= 0);
+                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(rmdir(q) >= 0);
+                assert_se(symlink("/etc/passwd", q) >= 0);
+                assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) == -EPERM);
+
+                assert_se(chown(p, 0, 0) >= 0);
+                assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) >= 0);
+        }
+
         assert_se(rm_rf(temp, REMOVE_ROOT|REMOVE_PHYSICAL) >= 0);
 }