daandemeyer / rpms / systemd

Forked from rpms/systemd 2 years ago
Clone
c62b8e
From 0e64363bb21a07fa318017ea8c90597db63a9545 Mon Sep 17 00:00:00 2001
c62b8e
From: David Tardon <dtardon@redhat.com>
c62b8e
Date: Thu, 3 Jan 2019 14:37:34 +0100
c62b8e
Subject: [PATCH] fs-util: add new CHASE_SAFE flag to chase_symlinks()
c62b8e
c62b8e
When the flag is specified we won't transition to a privilege-owned
c62b8e
file or directory from an unprivileged-owned one. This is useful when
c62b8e
privileged code wants to load data from a file unprivileged users have
c62b8e
write access to, and validates the ownership, but want's to make sure
c62b8e
that no symlink games are played to read a root-owned system file
c62b8e
belonging to a different context.
c62b8e
c62b8e
(cherry picked from commit f14f1806e329fe92d01f15c22a384702f0cb4ae0)
c62b8e
c62b8e
Related: #1663143
c62b8e
---
c62b8e
 src/shared/util.c    | 43 +++++++++++++++++++++++++++++++++++++++++++
c62b8e
 src/shared/util.h    |  7 ++++---
c62b8e
 src/test/test-util.c | 26 ++++++++++++++++++++++++++
c62b8e
 3 files changed, 73 insertions(+), 3 deletions(-)
c62b8e
c62b8e
diff --git a/src/shared/util.c b/src/shared/util.c
c62b8e
index 385551f2b3..fc4887920f 100644
c62b8e
--- a/src/shared/util.c
c62b8e
+++ b/src/shared/util.c
c62b8e
@@ -9203,10 +9203,22 @@ int fd_is_fs_type(int fd, statfs_f_type_t magic_value) {
c62b8e
         return is_fs_type(&s, magic_value);
c62b8e
 }
c62b8e
 
c62b8e
+static bool safe_transition(const struct stat *a, const struct stat *b) {
c62b8e
+        /* Returns true if the transition from a to b is safe, i.e. that we never transition from unprivileged to
c62b8e
+         * privileged files or directories. Why bother? So that unprivileged code can't symlink to privileged files
c62b8e
+         * making us believe we read something safe even though it isn't safe in the specific context we open it in. */
c62b8e
+
c62b8e
+        if (a->st_uid == 0) /* Transitioning from privileged to unprivileged is always fine */
c62b8e
+                return true;
c62b8e
+
c62b8e
+        return a->st_uid == b->st_uid; /* Otherwise we need to stay within the same UID */
c62b8e
+}
c62b8e
+
c62b8e
 int chase_symlinks(const char *path, const char *original_root, unsigned flags, char **ret) {
c62b8e
         _cleanup_free_ char *buffer = NULL, *done = NULL, *root = NULL;
c62b8e
         _cleanup_close_ int fd = -1;
c62b8e
         unsigned max_follow = 32; /* how many symlinks to follow before giving up and returning ELOOP */
c62b8e
+        struct stat previous_stat;
c62b8e
         bool exists = true;
c62b8e
         char *todo;
c62b8e
         int r;
c62b8e
@@ -9250,6 +9262,11 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
c62b8e
         if (fd < 0)
c62b8e
                 return -errno;
c62b8e
 
c62b8e
+        if (flags & CHASE_SAFE) {
c62b8e
+                if (fstat(fd, &previous_stat) < 0)
c62b8e
+                        return -errno;
c62b8e
+        }
c62b8e
+
c62b8e
         todo = buffer;
c62b8e
         for (;;) {
c62b8e
                 _cleanup_free_ char *first = NULL;
c62b8e
@@ -9313,6 +9330,16 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
c62b8e
                         if (fd_parent < 0)
c62b8e
                                 return -errno;
c62b8e
 
c62b8e
+                        if (flags & CHASE_SAFE) {
c62b8e
+                                if (fstat(fd_parent, &st) < 0)
c62b8e
+                                        return -errno;
c62b8e
+
c62b8e
+                                if (!safe_transition(&previous_stat, &st))
c62b8e
+                                        return -EPERM;
c62b8e
+
c62b8e
+                                previous_stat = st;
c62b8e
+                        }
c62b8e
+
c62b8e
                         safe_close(fd);
c62b8e
                         fd = fd_parent;
c62b8e
 
c62b8e
@@ -9347,6 +9374,12 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
c62b8e
 
c62b8e
                 if (fstat(child, &st) < 0)
c62b8e
                         return -errno;
c62b8e
+                if ((flags & CHASE_SAFE) &&
c62b8e
+                    !safe_transition(&previous_stat, &st))
c62b8e
+                        return -EPERM;
c62b8e
+
c62b8e
+                previous_stat = st;
c62b8e
+
c62b8e
                 if ((flags & CHASE_NO_AUTOFS) &&
c62b8e
                     fd_is_fs_type(child, AUTOFS_SUPER_MAGIC) > 0)
c62b8e
                         return -EREMOTE;
c62b8e
@@ -9379,6 +9412,16 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
c62b8e
 
c62b8e
                                 free(done);
c62b8e
 
c62b8e
+                                if (flags & CHASE_SAFE) {
c62b8e
+                                        if (fstat(fd, &st) < 0)
c62b8e
+                                                return -errno;
c62b8e
+
c62b8e
+                                        if (!safe_transition(&previous_stat, &st))
c62b8e
+                                                return -EPERM;
c62b8e
+
c62b8e
+                                        previous_stat = st;
c62b8e
+                                }
c62b8e
+
c62b8e
                                 /* Note that we do not revalidate the root, we take it as is. */
c62b8e
                                 if (isempty(root))
c62b8e
                                         done = NULL;
c62b8e
diff --git a/src/shared/util.h b/src/shared/util.h
c62b8e
index 915c7439e8..fa3e2e3009 100644
c62b8e
--- a/src/shared/util.h
c62b8e
+++ b/src/shared/util.h
c62b8e
@@ -1157,9 +1157,10 @@ bool is_fs_type(const struct statfs *s, statfs_f_type_t magic_value) _pure_;
c62b8e
 int fd_is_fs_type(int fd, statfs_f_type_t magic_value);
c62b8e
 
c62b8e
 enum {
c62b8e
-        CHASE_PREFIX_ROOT = 1,   /* If set, the specified path will be prefixed by the specified root before beginning the iteration */
c62b8e
-        CHASE_NONEXISTENT = 2,   /* If set, it's OK if the path doesn't actually exist. */
c62b8e
-        CHASE_NO_AUTOFS = 4,     /* If set, return -EREMOTE if autofs mount point found */
c62b8e
+        CHASE_PREFIX_ROOT = 1U << 0,   /* If set, the specified path will be prefixed by the specified root before beginning the iteration */
c62b8e
+        CHASE_NONEXISTENT = 1U << 1,   /* If set, it's OK if the path doesn't actually exist. */
c62b8e
+        CHASE_NO_AUTOFS   = 1U << 2,   /* If set, return -EREMOTE if autofs mount point found */
c62b8e
+        CHASE_SAFE        = 1U << 3,   /* If set, return EPERM if we ever traverse from unprivileged to privileged files or directories */
c62b8e
 };
c62b8e
 
c62b8e
 int chase_symlinks(const char *path_with_prefix, const char *root, unsigned flags, char **ret);
c62b8e
diff --git a/src/test/test-util.c b/src/test/test-util.c
c62b8e
index 397c45a9f4..e5a646ec20 100644
c62b8e
--- a/src/test/test-util.c
c62b8e
+++ b/src/test/test-util.c
c62b8e
@@ -2113,6 +2113,32 @@ static void test_chase_symlinks(void) {
c62b8e
         r = chase_symlinks(p, NULL, 0, &result);
c62b8e
         assert_se(r == -ENOENT);
c62b8e
 
c62b8e
+        if (geteuid() == 0) {
c62b8e
+                p = strjoina(temp, "/priv1");
c62b8e
+                assert_se(mkdir(p, 0755) >= 0);
c62b8e
+
c62b8e
+                q = strjoina(p, "/priv2");
c62b8e
+                assert_se(mkdir(q, 0755) >= 0);
c62b8e
+
c62b8e
+                assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) >= 0);
c62b8e
+
c62b8e
+                assert_se(chown(q, 65534, 65534) >= 0);
c62b8e
+                assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) >= 0);
c62b8e
+
c62b8e
+                assert_se(chown(p, 65534, 65534) >= 0);
c62b8e
+                assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) >= 0);
c62b8e
+
c62b8e
+                assert_se(chown(q, 0, 0) >= 0);
c62b8e
+                assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) == -EPERM);
c62b8e
+
c62b8e
+                assert_se(rmdir(q) >= 0);
c62b8e
+                assert_se(symlink("/etc/passwd", q) >= 0);
c62b8e
+                assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) == -EPERM);
c62b8e
+
c62b8e
+                assert_se(chown(p, 0, 0) >= 0);
c62b8e
+                assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) >= 0);
c62b8e
+        }
c62b8e
+
c62b8e
         assert_se(rm_rf_dangerous(temp, false, true, false) >= 0);
c62b8e
 }
c62b8e