daandemeyer / rpms / systemd

Forked from rpms/systemd 2 years ago
Clone
36e8a3
From 107d75ca9394481bd045385fc45f2ee65b30ad16 Mon Sep 17 00:00:00 2001
36e8a3
From: Lennart Poettering <lennart@poettering.net>
36e8a3
Date: Fri, 19 Oct 2018 11:26:59 +0200
36e8a3
Subject: [PATCH] chown-recursive: let's rework the recursive logic to use
36e8a3
 O_PATH
36e8a3
36e8a3
That way we can pin a specific inode and analyze it and manipulate it
36e8a3
without it being swapped out beneath our hands.
36e8a3
36e8a3
Fixes a vulnerability originally found by Jann Horn from Google.
36e8a3
36e8a3
CVE-2018-15687
36e8a3
LP: #1796692
36e8a3
https://bugzilla.redhat.com/show_bug.cgi?id=1639076
36e8a3
36e8a3
(cherry-picked from commit 5de6cce58b3e8b79239b6e83653459d91af6e57c)
36e8a3
36e8a3
Resolves: #1643368
36e8a3
---
36e8a3
 src/core/chown-recursive.c | 146 ++++++++++++++++++-------------------
36e8a3
 1 file changed, 70 insertions(+), 76 deletions(-)
36e8a3
36e8a3
diff --git a/src/core/chown-recursive.c b/src/core/chown-recursive.c
4bff0a
index c4794501c2..27c64489b5 100644
36e8a3
--- a/src/core/chown-recursive.c
36e8a3
+++ b/src/core/chown-recursive.c
36e8a3
@@ -1,17 +1,19 @@
36e8a3
 /* SPDX-License-Identifier: LGPL-2.1+ */
36e8a3
 
36e8a3
-#include <sys/types.h>
36e8a3
-#include <sys/stat.h>
36e8a3
 #include <fcntl.h>
36e8a3
+#include <sys/stat.h>
36e8a3
+#include <sys/types.h>
36e8a3
 
36e8a3
-#include "user-util.h"
36e8a3
-#include "macro.h"
36e8a3
-#include "fd-util.h"
36e8a3
-#include "dirent-util.h"
36e8a3
 #include "chown-recursive.h"
36e8a3
+#include "dirent-util.h"
36e8a3
+#include "fd-util.h"
36e8a3
+#include "macro.h"
36e8a3
+#include "stdio-util.h"
36e8a3
+#include "strv.h"
36e8a3
+#include "user-util.h"
36e8a3
 
36e8a3
-static int chown_one(int fd, const char *name, const struct stat *st, uid_t uid, gid_t gid) {
36e8a3
-        int r;
36e8a3
+static int chown_one(int fd, const struct stat *st, uid_t uid, gid_t gid) {
36e8a3
+        char procfs_path[STRLEN("/proc/self/fd/") + DECIMAL_STR_MAX(int) + 1];
36e8a3
 
36e8a3
         assert(fd >= 0);
36e8a3
         assert(st);
36e8a3
@@ -20,90 +22,82 @@ static int chown_one(int fd, const char *name, const struct stat *st, uid_t uid,
36e8a3
             (!gid_is_valid(gid) || st->st_gid == gid))
36e8a3
                 return 0;
36e8a3
 
36e8a3
-        if (name)
36e8a3
-                r = fchownat(fd, name, uid, gid, AT_SYMLINK_NOFOLLOW);
36e8a3
-        else
36e8a3
-                r = fchown(fd, uid, gid);
36e8a3
-        if (r < 0)
36e8a3
-                return -errno;
36e8a3
+        /* We change ownership through the /proc/self/fd/%i path, so that we have a stable reference that works with
36e8a3
+         * O_PATH. (Note: fchown() and fchmod() do not work with O_PATH, the kernel refuses that. */
36e8a3
+        xsprintf(procfs_path, "/proc/self/fd/%i", fd);
36e8a3
 
36e8a3
-        /* The linux kernel alters the mode in some cases of chown(). Let's undo this. */
36e8a3
-        if (name) {
36e8a3
-                if (!S_ISLNK(st->st_mode))
36e8a3
-                        r = fchmodat(fd, name, st->st_mode, 0);
36e8a3
-                else /* There's currently no AT_SYMLINK_NOFOLLOW for fchmodat() */
36e8a3
-                        r = 0;
36e8a3
-        } else
36e8a3
-                r = fchmod(fd, st->st_mode);
36e8a3
-        if (r < 0)
36e8a3
+        if (chown(procfs_path, uid, gid) < 0)
36e8a3
                 return -errno;
36e8a3
 
36e8a3
+        /* The linux kernel alters the mode in some cases of chown(). Let's undo this. We do this only for non-symlinks
36e8a3
+         * however. That's because for symlinks the access mode is ignored anyway and because on some kernels/file
36e8a3
+         * systems trying to change the access mode will succeed but has no effect while on others it actively
36e8a3
+         * fails. */
36e8a3
+        if (!S_ISLNK(st->st_mode))
36e8a3
+                if (chmod(procfs_path, st->st_mode & 07777) < 0)
36e8a3
+                        return -errno;
36e8a3
+
36e8a3
         return 1;
36e8a3
 }
36e8a3
 
36e8a3
 static int chown_recursive_internal(int fd, const struct stat *st, uid_t uid, gid_t gid) {
36e8a3
+        _cleanup_closedir_ DIR *d = NULL;
36e8a3
         bool changed = false;
36e8a3
+        struct dirent *de;
36e8a3
         int r;
36e8a3
 
36e8a3
         assert(fd >= 0);
36e8a3
         assert(st);
36e8a3
 
36e8a3
-        if (S_ISDIR(st->st_mode)) {
36e8a3
-                _cleanup_closedir_ DIR *d = NULL;
36e8a3
-                struct dirent *de;
36e8a3
-
36e8a3
-                d = fdopendir(fd);
36e8a3
-                if (!d) {
36e8a3
-                        r = -errno;
36e8a3
-                        goto finish;
36e8a3
-                }
36e8a3
-                fd = -1;
36e8a3
-
36e8a3
-                FOREACH_DIRENT_ALL(de, d, r = -errno; goto finish) {
36e8a3
-                        struct stat fst;
36e8a3
-
36e8a3
-                        if (dot_or_dot_dot(de->d_name))
36e8a3
-                                continue;
36e8a3
-
36e8a3
-                        if (fstatat(dirfd(d), de->d_name, &fst, AT_SYMLINK_NOFOLLOW) < 0) {
36e8a3
-                                r = -errno;
36e8a3
-                                goto finish;
36e8a3
-                        }
36e8a3
-
36e8a3
-                        if (S_ISDIR(fst.st_mode)) {
36e8a3
-                                int subdir_fd;
36e8a3
-
36e8a3
-                                subdir_fd = openat(dirfd(d), de->d_name, O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC|O_NOFOLLOW|O_NOATIME);
36e8a3
-                                if (subdir_fd < 0) {
36e8a3
-                                        r = -errno;
36e8a3
-                                        goto finish;
36e8a3
-                                }
36e8a3
-
36e8a3
-                                r = chown_recursive_internal(subdir_fd, &fst, uid, gid);
36e8a3
-                                if (r < 0)
36e8a3
-                                        goto finish;
36e8a3
-                                if (r > 0)
36e8a3
-                                        changed = true;
36e8a3
-                        } else {
36e8a3
-                                r = chown_one(dirfd(d), de->d_name, &fst, uid, gid);
36e8a3
-                                if (r < 0)
36e8a3
-                                        goto finish;
36e8a3
-                                if (r > 0)
36e8a3
-                                        changed = true;
36e8a3
-                        }
36e8a3
+        d = fdopendir(fd);
36e8a3
+        if (!d) {
36e8a3
+                safe_close(fd);
36e8a3
+                return -errno;
36e8a3
+        }
36e8a3
+
36e8a3
+        FOREACH_DIRENT_ALL(de, d, return -errno) {
36e8a3
+                _cleanup_close_ int path_fd = -1;
36e8a3
+                struct stat fst;
36e8a3
+
36e8a3
+                if (dot_or_dot_dot(de->d_name))
36e8a3
+                        continue;
36e8a3
+
36e8a3
+                /* Let's pin the child inode we want to fix now with an O_PATH fd, so that it cannot be swapped out
36e8a3
+                 * while we manipulate it. */
36e8a3
+                path_fd = openat(dirfd(d), de->d_name, O_PATH|O_CLOEXEC|O_NOFOLLOW);
36e8a3
+                if (path_fd < 0)
36e8a3
+                        return -errno;
36e8a3
+
36e8a3
+                if (fstat(path_fd, &fst) < 0)
36e8a3
+                        return -errno;
36e8a3
+
36e8a3
+                if (S_ISDIR(fst.st_mode)) {
36e8a3
+                        int subdir_fd;
36e8a3
+
36e8a3
+                        /* Convert it to a "real" (i.e. non-O_PATH) fd now */
36e8a3
+                        subdir_fd = fd_reopen(path_fd, O_RDONLY|O_CLOEXEC|O_NOATIME);
36e8a3
+                        if (subdir_fd < 0)
36e8a3
+                                return subdir_fd;
36e8a3
+
36e8a3
+                        r = chown_recursive_internal(subdir_fd, &fst, uid, gid); /* takes possession of subdir_fd even on failure */
36e8a3
+                        if (r < 0)
36e8a3
+                                return r;
36e8a3
+                        if (r > 0)
36e8a3
+                                changed = true;
36e8a3
+                } else {
36e8a3
+                        r = chown_one(path_fd, &fst, uid, gid);
36e8a3
+                        if (r < 0)
36e8a3
+                                return r;
36e8a3
+                        if (r > 0)
36e8a3
+                                changed = true;
36e8a3
                 }
36e8a3
+        }
36e8a3
 
36e8a3
-                r = chown_one(dirfd(d), NULL, st, uid, gid);
36e8a3
-        } else
36e8a3
-                r = chown_one(fd, NULL, st, uid, gid);
36e8a3
+        r = chown_one(dirfd(d), st, uid, gid);
36e8a3
         if (r < 0)
36e8a3
-                goto finish;
36e8a3
+                return r;
36e8a3
 
36e8a3
-        r = r > 0 || changed;
36e8a3
-
36e8a3
-finish:
36e8a3
-        safe_close(fd);
36e8a3
-        return r;
36e8a3
+        return r > 0 || changed;
36e8a3
 }
36e8a3
 
36e8a3
 int path_chown_recursive(const char *path, uid_t uid, gid_t gid) {
36e8a3
@@ -111,7 +105,7 @@ int path_chown_recursive(const char *path, uid_t uid, gid_t gid) {
36e8a3
         struct stat st;
36e8a3
         int r;
36e8a3
 
36e8a3
-        fd = open(path, O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC|O_NOFOLLOW|O_NOATIME);
36e8a3
+        fd = open(path, O_RDONLY|O_DIRECTORY|O_CLOEXEC|O_NOFOLLOW|O_NOATIME);
36e8a3
         if (fd < 0)
36e8a3
                 return -errno;
36e8a3