Brad Bishop | 1a4b7ee | 2018-12-16 17:11:34 -0800 | [diff] [blame] | 1 | From 2da8ba3f507345d0401ea9d7191fa16ffa560ebc Mon Sep 17 00:00:00 2001 |
| 2 | From: Lennart Poettering <lennart@poettering.net> |
| 3 | Date: Fri, 19 Oct 2018 11:26:59 +0200 |
| 4 | Subject: [PATCH] chown-recursive: let's rework the recursive logic to use |
| 5 | O_PATH |
| 6 | |
| 7 | That way we can pin a specific inode and analyze it and manipulate it |
| 8 | without it being swapped out beneath our hands. |
| 9 | |
| 10 | Fixes a vulnerability originally found by Jann Horn from Google. |
| 11 | |
| 12 | CVE-2018-15687 |
| 13 | LP: #1796692 |
| 14 | https://bugzilla.redhat.com/show_bug.cgi?id=1639076 |
| 15 | |
| 16 | (cherry picked from commit 5de6cce58b3e8b79239b6e83653459d91af6e57c) |
| 17 | |
| 18 | CVE: CVE-2018-15687 |
| 19 | Upstream-Status: Backport |
| 20 | |
| 21 | Signed-off-by: Chen Qi <Qi.Chen@windriver.com> |
| 22 | --- |
| 23 | src/core/chown-recursive.c | 146 ++++++++++++++++++++++----------------------- |
| 24 | 1 file changed, 70 insertions(+), 76 deletions(-) |
| 25 | |
| 26 | diff --git a/src/core/chown-recursive.c b/src/core/chown-recursive.c |
| 27 | index c479450..27c6448 100644 |
| 28 | --- a/src/core/chown-recursive.c |
| 29 | +++ b/src/core/chown-recursive.c |
| 30 | @@ -1,17 +1,19 @@ |
| 31 | /* SPDX-License-Identifier: LGPL-2.1+ */ |
| 32 | |
| 33 | -#include <sys/types.h> |
| 34 | -#include <sys/stat.h> |
| 35 | #include <fcntl.h> |
| 36 | +#include <sys/stat.h> |
| 37 | +#include <sys/types.h> |
| 38 | |
| 39 | -#include "user-util.h" |
| 40 | -#include "macro.h" |
| 41 | -#include "fd-util.h" |
| 42 | -#include "dirent-util.h" |
| 43 | #include "chown-recursive.h" |
| 44 | +#include "dirent-util.h" |
| 45 | +#include "fd-util.h" |
| 46 | +#include "macro.h" |
| 47 | +#include "stdio-util.h" |
| 48 | +#include "strv.h" |
| 49 | +#include "user-util.h" |
| 50 | |
| 51 | -static int chown_one(int fd, const char *name, const struct stat *st, uid_t uid, gid_t gid) { |
| 52 | - int r; |
| 53 | +static int chown_one(int fd, const struct stat *st, uid_t uid, gid_t gid) { |
| 54 | + char procfs_path[STRLEN("/proc/self/fd/") + DECIMAL_STR_MAX(int) + 1]; |
| 55 | |
| 56 | assert(fd >= 0); |
| 57 | assert(st); |
| 58 | @@ -20,90 +22,82 @@ static int chown_one(int fd, const char *name, const struct stat *st, uid_t uid, |
| 59 | (!gid_is_valid(gid) || st->st_gid == gid)) |
| 60 | return 0; |
| 61 | |
| 62 | - if (name) |
| 63 | - r = fchownat(fd, name, uid, gid, AT_SYMLINK_NOFOLLOW); |
| 64 | - else |
| 65 | - r = fchown(fd, uid, gid); |
| 66 | - if (r < 0) |
| 67 | - return -errno; |
| 68 | + /* We change ownership through the /proc/self/fd/%i path, so that we have a stable reference that works with |
| 69 | + * O_PATH. (Note: fchown() and fchmod() do not work with O_PATH, the kernel refuses that. */ |
| 70 | + xsprintf(procfs_path, "/proc/self/fd/%i", fd); |
| 71 | |
| 72 | - /* The linux kernel alters the mode in some cases of chown(). Let's undo this. */ |
| 73 | - if (name) { |
| 74 | - if (!S_ISLNK(st->st_mode)) |
| 75 | - r = fchmodat(fd, name, st->st_mode, 0); |
| 76 | - else /* There's currently no AT_SYMLINK_NOFOLLOW for fchmodat() */ |
| 77 | - r = 0; |
| 78 | - } else |
| 79 | - r = fchmod(fd, st->st_mode); |
| 80 | - if (r < 0) |
| 81 | + if (chown(procfs_path, uid, gid) < 0) |
| 82 | return -errno; |
| 83 | |
| 84 | + /* The linux kernel alters the mode in some cases of chown(). Let's undo this. We do this only for non-symlinks |
| 85 | + * however. That's because for symlinks the access mode is ignored anyway and because on some kernels/file |
| 86 | + * systems trying to change the access mode will succeed but has no effect while on others it actively |
| 87 | + * fails. */ |
| 88 | + if (!S_ISLNK(st->st_mode)) |
| 89 | + if (chmod(procfs_path, st->st_mode & 07777) < 0) |
| 90 | + return -errno; |
| 91 | + |
| 92 | return 1; |
| 93 | } |
| 94 | |
| 95 | static int chown_recursive_internal(int fd, const struct stat *st, uid_t uid, gid_t gid) { |
| 96 | + _cleanup_closedir_ DIR *d = NULL; |
| 97 | bool changed = false; |
| 98 | + struct dirent *de; |
| 99 | int r; |
| 100 | |
| 101 | assert(fd >= 0); |
| 102 | assert(st); |
| 103 | |
| 104 | - if (S_ISDIR(st->st_mode)) { |
| 105 | - _cleanup_closedir_ DIR *d = NULL; |
| 106 | - struct dirent *de; |
| 107 | - |
| 108 | - d = fdopendir(fd); |
| 109 | - if (!d) { |
| 110 | - r = -errno; |
| 111 | - goto finish; |
| 112 | - } |
| 113 | - fd = -1; |
| 114 | - |
| 115 | - FOREACH_DIRENT_ALL(de, d, r = -errno; goto finish) { |
| 116 | - struct stat fst; |
| 117 | - |
| 118 | - if (dot_or_dot_dot(de->d_name)) |
| 119 | - continue; |
| 120 | - |
| 121 | - if (fstatat(dirfd(d), de->d_name, &fst, AT_SYMLINK_NOFOLLOW) < 0) { |
| 122 | - r = -errno; |
| 123 | - goto finish; |
| 124 | - } |
| 125 | - |
| 126 | - if (S_ISDIR(fst.st_mode)) { |
| 127 | - int subdir_fd; |
| 128 | - |
| 129 | - subdir_fd = openat(dirfd(d), de->d_name, O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC|O_NOFOLLOW|O_NOATIME); |
| 130 | - if (subdir_fd < 0) { |
| 131 | - r = -errno; |
| 132 | - goto finish; |
| 133 | - } |
| 134 | - |
| 135 | - r = chown_recursive_internal(subdir_fd, &fst, uid, gid); |
| 136 | - if (r < 0) |
| 137 | - goto finish; |
| 138 | - if (r > 0) |
| 139 | - changed = true; |
| 140 | - } else { |
| 141 | - r = chown_one(dirfd(d), de->d_name, &fst, uid, gid); |
| 142 | - if (r < 0) |
| 143 | - goto finish; |
| 144 | - if (r > 0) |
| 145 | - changed = true; |
| 146 | - } |
| 147 | + d = fdopendir(fd); |
| 148 | + if (!d) { |
| 149 | + safe_close(fd); |
| 150 | + return -errno; |
| 151 | + } |
| 152 | + |
| 153 | + FOREACH_DIRENT_ALL(de, d, return -errno) { |
| 154 | + _cleanup_close_ int path_fd = -1; |
| 155 | + struct stat fst; |
| 156 | + |
| 157 | + if (dot_or_dot_dot(de->d_name)) |
| 158 | + continue; |
| 159 | + |
| 160 | + /* Let's pin the child inode we want to fix now with an O_PATH fd, so that it cannot be swapped out |
| 161 | + * while we manipulate it. */ |
| 162 | + path_fd = openat(dirfd(d), de->d_name, O_PATH|O_CLOEXEC|O_NOFOLLOW); |
| 163 | + if (path_fd < 0) |
| 164 | + return -errno; |
| 165 | + |
| 166 | + if (fstat(path_fd, &fst) < 0) |
| 167 | + return -errno; |
| 168 | + |
| 169 | + if (S_ISDIR(fst.st_mode)) { |
| 170 | + int subdir_fd; |
| 171 | + |
| 172 | + /* Convert it to a "real" (i.e. non-O_PATH) fd now */ |
| 173 | + subdir_fd = fd_reopen(path_fd, O_RDONLY|O_CLOEXEC|O_NOATIME); |
| 174 | + if (subdir_fd < 0) |
| 175 | + return subdir_fd; |
| 176 | + |
| 177 | + r = chown_recursive_internal(subdir_fd, &fst, uid, gid); /* takes possession of subdir_fd even on failure */ |
| 178 | + if (r < 0) |
| 179 | + return r; |
| 180 | + if (r > 0) |
| 181 | + changed = true; |
| 182 | + } else { |
| 183 | + r = chown_one(path_fd, &fst, uid, gid); |
| 184 | + if (r < 0) |
| 185 | + return r; |
| 186 | + if (r > 0) |
| 187 | + changed = true; |
| 188 | } |
| 189 | + } |
| 190 | |
| 191 | - r = chown_one(dirfd(d), NULL, st, uid, gid); |
| 192 | - } else |
| 193 | - r = chown_one(fd, NULL, st, uid, gid); |
| 194 | + r = chown_one(dirfd(d), st, uid, gid); |
| 195 | if (r < 0) |
| 196 | - goto finish; |
| 197 | + return r; |
| 198 | |
| 199 | - r = r > 0 || changed; |
| 200 | - |
| 201 | -finish: |
| 202 | - safe_close(fd); |
| 203 | - return r; |
| 204 | + return r > 0 || changed; |
| 205 | } |
| 206 | |
| 207 | int path_chown_recursive(const char *path, uid_t uid, gid_t gid) { |
| 208 | @@ -111,7 +105,7 @@ int path_chown_recursive(const char *path, uid_t uid, gid_t gid) { |
| 209 | struct stat st; |
| 210 | int r; |
| 211 | |
| 212 | - fd = open(path, O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC|O_NOFOLLOW|O_NOATIME); |
| 213 | + fd = open(path, O_RDONLY|O_DIRECTORY|O_CLOEXEC|O_NOFOLLOW|O_NOATIME); |
| 214 | if (fd < 0) |
| 215 | return -errno; |
| 216 | |
| 217 | -- |
| 218 | 2.7.4 |
| 219 | |