1From 1d5e0e9910500f3c3584485f77bfc35e601036e3 Mon Sep 17 00:00:00 2001 2From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl> 3Date: Mon, 28 Nov 2022 12:12:55 +0100 4Subject: [PATCH 2/2] coredump: do not allow user to access coredumps with 5 changed uid/gid/capabilities 6 7When the user starts a program which elevates its permissions via setuid, 8setgid, or capabilities set on the file, it may access additional information 9which would then be visible in the coredump. We shouldn't make the the coredump 10visible to the user in such cases. 11 12Reported-by: Matthias Gerstner <mgerstner@suse.de> 13 14This reads the /proc/<pid>/auxv file and attaches it to the process metadata as 15PROC_AUXV. Before the coredump is submitted, it is parsed and if either 16at_secure was set (which the kernel will do for processes that are setuid, 17setgid, or setcap), or if the effective uid/gid don't match uid/gid, the file 18is not made accessible to the user. If we can't access this data, we assume the 19file should not be made accessible either. In principle we could also access 20the auxv data from a note in the core file, but that is much more complex and 21it seems better to use the stand-alone file that is provided by the kernel. 22 23Attaching auxv is both convient for this patch (because this way it's passed 24between the stages along with other fields), but I think it makes sense to save 25it in general. 26 27We use the information early in the core file to figure out if the program was 2832-bit or 64-bit and its endianness. This way we don't need heuristics to guess 29whether the format of the auxv structure. This test might reject some cases on 30fringe architecutes. But the impact would be limited: we just won't grant the 31user permissions to view the coredump file. If people report that we're missing 32some cases, we can always enhance this to support more architectures. 33 34I tested auxv parsing on amd64, 32-bit program on amd64, arm64, arm32, and 35ppc64el, but not the whole coredump handling. 36 37(cherry picked from commit 3e4d0f6cf99f8677edd6a237382a65bfe758de03) 38(cherry picked from commit 9b75a3d0502d6741c8ecb7175794345f8eb3827c) 39(cherry picked from commit efca5283dc791a07171f80eef84e14fdb58fad57) 40 41CVE: CVE-2022-4415 42Upstream-Status: Backport [https://github.com/systemd/systemd-stable/commit/1d5e0e9910500f3c3584485f77bfc35e601036e3] 43 44Signed-off-by: Peter Marko <peter.marko@siemens.com> 45--- 46 src/basic/io-util.h | 9 ++ 47 src/coredump/coredump.c | 196 +++++++++++++++++++++++++++++++++++++--- 48 2 files changed, 192 insertions(+), 13 deletions(-) 49 50diff --git a/src/basic/io-util.h b/src/basic/io-util.h 51index 39728e06bc..3afb134266 100644 52--- a/src/basic/io-util.h 53+++ b/src/basic/io-util.h 54@@ -91,7 +91,16 @@ struct iovec_wrapper *iovw_new(void); 55 struct iovec_wrapper *iovw_free(struct iovec_wrapper *iovw); 56 struct iovec_wrapper *iovw_free_free(struct iovec_wrapper *iovw); 57 void iovw_free_contents(struct iovec_wrapper *iovw, bool free_vectors); 58+ 59 int iovw_put(struct iovec_wrapper *iovw, void *data, size_t len); 60+static inline int iovw_consume(struct iovec_wrapper *iovw, void *data, size_t len) { 61+ /* Move data into iovw or free on error */ 62+ int r = iovw_put(iovw, data, len); 63+ if (r < 0) 64+ free(data); 65+ return r; 66+} 67+ 68 int iovw_put_string_field(struct iovec_wrapper *iovw, const char *field, const char *value); 69 int iovw_put_string_field_free(struct iovec_wrapper *iovw, const char *field, char *value); 70 void iovw_rebase(struct iovec_wrapper *iovw, char *old, char *new); 71diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c 72index 8295b03ac7..79280ab986 100644 73--- a/src/coredump/coredump.c 74+++ b/src/coredump/coredump.c 75@@ -4,6 +4,7 @@ 76 #include <stdio.h> 77 #include <sys/prctl.h> 78 #include <sys/statvfs.h> 79+#include <sys/auxv.h> 80 #include <sys/xattr.h> 81 #include <unistd.h> 82 83@@ -99,6 +100,7 @@ enum { 84 85 META_EXE = _META_MANDATORY_MAX, 86 META_UNIT, 87+ META_PROC_AUXV, 88 _META_MAX 89 }; 90 91@@ -113,10 +115,12 @@ static const char * const meta_field_names[_META_MAX] = { 92 [META_COMM] = "COREDUMP_COMM=", 93 [META_EXE] = "COREDUMP_EXE=", 94 [META_UNIT] = "COREDUMP_UNIT=", 95+ [META_PROC_AUXV] = "COREDUMP_PROC_AUXV=", 96 }; 97 98 typedef struct Context { 99 const char *meta[_META_MAX]; 100+ size_t meta_size[_META_MAX]; 101 pid_t pid; 102 bool is_pid1; 103 bool is_journald; 104@@ -178,13 +182,16 @@ static uint64_t storage_size_max(void) { 105 return 0; 106 } 107 108-static int fix_acl(int fd, uid_t uid) { 109+static int fix_acl(int fd, uid_t uid, bool allow_user) { 110+ assert(fd >= 0); 111+ assert(uid_is_valid(uid)); 112 113 #if HAVE_ACL 114 int r; 115 116- assert(fd >= 0); 117- assert(uid_is_valid(uid)); 118+ /* We don't allow users to read coredumps if the uid or capabilities were changed. */ 119+ if (!allow_user) 120+ return 0; 121 122 if (uid_is_system(uid) || uid_is_dynamic(uid) || uid == UID_NOBODY) 123 return 0; 124@@ -244,7 +251,8 @@ static int fix_permissions( 125 const char *filename, 126 const char *target, 127 const Context *context, 128- uid_t uid) { 129+ uid_t uid, 130+ bool allow_user) { 131 132 int r; 133 134@@ -254,7 +262,7 @@ static int fix_permissions( 135 136 /* Ignore errors on these */ 137 (void) fchmod(fd, 0640); 138- (void) fix_acl(fd, uid); 139+ (void) fix_acl(fd, uid, allow_user); 140 (void) fix_xattr(fd, context); 141 142 r = fsync_full(fd); 143@@ -324,6 +332,153 @@ static int make_filename(const Context *context, char **ret) { 144 return 0; 145 } 146 147+static int parse_auxv64( 148+ const uint64_t *auxv, 149+ size_t size_bytes, 150+ int *at_secure, 151+ uid_t *uid, 152+ uid_t *euid, 153+ gid_t *gid, 154+ gid_t *egid) { 155+ 156+ assert(auxv || size_bytes == 0); 157+ 158+ if (size_bytes % (2 * sizeof(uint64_t)) != 0) 159+ return log_warning_errno(SYNTHETIC_ERRNO(EIO), "Incomplete auxv structure (%zu bytes).", size_bytes); 160+ 161+ size_t words = size_bytes / sizeof(uint64_t); 162+ 163+ /* Note that we set output variables even on error. */ 164+ 165+ for (size_t i = 0; i + 1 < words; i += 2) 166+ switch (auxv[i]) { 167+ case AT_SECURE: 168+ *at_secure = auxv[i + 1] != 0; 169+ break; 170+ case AT_UID: 171+ *uid = auxv[i + 1]; 172+ break; 173+ case AT_EUID: 174+ *euid = auxv[i + 1]; 175+ break; 176+ case AT_GID: 177+ *gid = auxv[i + 1]; 178+ break; 179+ case AT_EGID: 180+ *egid = auxv[i + 1]; 181+ break; 182+ case AT_NULL: 183+ if (auxv[i + 1] != 0) 184+ goto error; 185+ return 0; 186+ } 187+ error: 188+ return log_warning_errno(SYNTHETIC_ERRNO(ENODATA), 189+ "AT_NULL terminator not found, cannot parse auxv structure."); 190+} 191+ 192+static int parse_auxv32( 193+ const uint32_t *auxv, 194+ size_t size_bytes, 195+ int *at_secure, 196+ uid_t *uid, 197+ uid_t *euid, 198+ gid_t *gid, 199+ gid_t *egid) { 200+ 201+ assert(auxv || size_bytes == 0); 202+ 203+ size_t words = size_bytes / sizeof(uint32_t); 204+ 205+ if (size_bytes % (2 * sizeof(uint32_t)) != 0) 206+ return log_warning_errno(SYNTHETIC_ERRNO(EIO), "Incomplete auxv structure (%zu bytes).", size_bytes); 207+ 208+ /* Note that we set output variables even on error. */ 209+ 210+ for (size_t i = 0; i + 1 < words; i += 2) 211+ switch (auxv[i]) { 212+ case AT_SECURE: 213+ *at_secure = auxv[i + 1] != 0; 214+ break; 215+ case AT_UID: 216+ *uid = auxv[i + 1]; 217+ break; 218+ case AT_EUID: 219+ *euid = auxv[i + 1]; 220+ break; 221+ case AT_GID: 222+ *gid = auxv[i + 1]; 223+ break; 224+ case AT_EGID: 225+ *egid = auxv[i + 1]; 226+ break; 227+ case AT_NULL: 228+ if (auxv[i + 1] != 0) 229+ goto error; 230+ return 0; 231+ } 232+ error: 233+ return log_warning_errno(SYNTHETIC_ERRNO(ENODATA), 234+ "AT_NULL terminator not found, cannot parse auxv structure."); 235+} 236+ 237+static int grant_user_access(int core_fd, const Context *context) { 238+ int at_secure = -1; 239+ uid_t uid = UID_INVALID, euid = UID_INVALID; 240+ uid_t gid = GID_INVALID, egid = GID_INVALID; 241+ int r; 242+ 243+ assert(core_fd >= 0); 244+ assert(context); 245+ 246+ if (!context->meta[META_PROC_AUXV]) 247+ return log_warning_errno(SYNTHETIC_ERRNO(ENODATA), "No auxv data, not adjusting permissions."); 248+ 249+ uint8_t elf[EI_NIDENT]; 250+ errno = 0; 251+ if (pread(core_fd, &elf, sizeof(elf), 0) != sizeof(elf)) 252+ return log_warning_errno(errno_or_else(EIO), 253+ "Failed to pread from coredump fd: %s", errno != 0 ? strerror_safe(errno) : "Unexpected EOF"); 254+ 255+ if (elf[EI_MAG0] != ELFMAG0 || 256+ elf[EI_MAG1] != ELFMAG1 || 257+ elf[EI_MAG2] != ELFMAG2 || 258+ elf[EI_MAG3] != ELFMAG3 || 259+ elf[EI_VERSION] != EV_CURRENT) 260+ return log_info_errno(SYNTHETIC_ERRNO(EUCLEAN), 261+ "Core file does not have ELF header, not adjusting permissions."); 262+ if (!IN_SET(elf[EI_CLASS], ELFCLASS32, ELFCLASS64) || 263+ !IN_SET(elf[EI_DATA], ELFDATA2LSB, ELFDATA2MSB)) 264+ return log_info_errno(SYNTHETIC_ERRNO(EUCLEAN), 265+ "Core file has strange ELF class, not adjusting permissions."); 266+ 267+ if ((elf[EI_DATA] == ELFDATA2LSB) != (__BYTE_ORDER == __LITTLE_ENDIAN)) 268+ return log_info_errno(SYNTHETIC_ERRNO(EUCLEAN), 269+ "Core file has non-native endianness, not adjusting permissions."); 270+ 271+ if (elf[EI_CLASS] == ELFCLASS64) 272+ r = parse_auxv64((const uint64_t*) context->meta[META_PROC_AUXV], 273+ context->meta_size[META_PROC_AUXV], 274+ &at_secure, &uid, &euid, &gid, &egid); 275+ else 276+ r = parse_auxv32((const uint32_t*) context->meta[META_PROC_AUXV], 277+ context->meta_size[META_PROC_AUXV], 278+ &at_secure, &uid, &euid, &gid, &egid); 279+ if (r < 0) 280+ return r; 281+ 282+ /* We allow access if we got all the data and at_secure is not set and 283+ * the uid/gid matches euid/egid. */ 284+ bool ret = 285+ at_secure == 0 && 286+ uid != UID_INVALID && euid != UID_INVALID && uid == euid && 287+ gid != GID_INVALID && egid != GID_INVALID && gid == egid; 288+ log_debug("Will %s access (uid="UID_FMT " euid="UID_FMT " gid="GID_FMT " egid="GID_FMT " at_secure=%s)", 289+ ret ? "permit" : "restrict", 290+ uid, euid, gid, egid, yes_no(at_secure)); 291+ return ret; 292+} 293+ 294 static int save_external_coredump( 295 const Context *context, 296 int input_fd, 297@@ -446,6 +601,8 @@ static int save_external_coredump( 298 context->meta[META_ARGV_PID], context->meta[META_COMM]); 299 truncated = r == 1; 300 301+ bool allow_user = grant_user_access(fd, context) > 0; 302+ 303 #if HAVE_COMPRESSION 304 if (arg_compress) { 305 _cleanup_(unlink_and_freep) char *tmp_compressed = NULL; 306@@ -483,7 +640,7 @@ static int save_external_coredump( 307 uncompressed_size += partial_uncompressed_size; 308 } 309 310- r = fix_permissions(fd_compressed, tmp_compressed, fn_compressed, context, uid); 311+ r = fix_permissions(fd_compressed, tmp_compressed, fn_compressed, context, uid, allow_user); 312 if (r < 0) 313 return r; 314 315@@ -510,7 +667,7 @@ static int save_external_coredump( 316 "SIZE_LIMIT=%zu", max_size, 317 "MESSAGE_ID=" SD_MESSAGE_TRUNCATED_CORE_STR); 318 319- r = fix_permissions(fd, tmp, fn, context, uid); 320+ r = fix_permissions(fd, tmp, fn, context, uid, allow_user); 321 if (r < 0) 322 return log_error_errno(r, "Failed to fix permissions and finalize coredump %s into %s: %m", coredump_tmpfile_name(tmp), fn); 323 324@@ -758,7 +915,7 @@ static int change_uid_gid(const Context *context) { 325 } 326 327 static int submit_coredump( 328- Context *context, 329+ const Context *context, 330 struct iovec_wrapper *iovw, 331 int input_fd) { 332 333@@ -919,16 +1076,15 @@ static int save_context(Context *context, const struct iovec_wrapper *iovw) { 334 struct iovec *iovec = iovw->iovec + n; 335 336 for (size_t i = 0; i < ELEMENTSOF(meta_field_names); i++) { 337- char *p; 338- 339 /* Note that these strings are NUL terminated, because we made sure that a 340 * trailing NUL byte is in the buffer, though not included in the iov_len 341 * count (see process_socket() and gather_pid_metadata_*()) */ 342 assert(((char*) iovec->iov_base)[iovec->iov_len] == 0); 343 344- p = startswith(iovec->iov_base, meta_field_names[i]); 345+ const char *p = startswith(iovec->iov_base, meta_field_names[i]); 346 if (p) { 347 context->meta[i] = p; 348+ context->meta_size[i] = iovec->iov_len - strlen(meta_field_names[i]); 349 count++; 350 break; 351 } 352@@ -1170,6 +1326,7 @@ static int gather_pid_metadata(struct iovec_wrapper *iovw, Context *context) { 353 uid_t owner_uid; 354 pid_t pid; 355 char *t; 356+ size_t size; 357 const char *p; 358 int r; 359 360@@ -1234,13 +1391,26 @@ static int gather_pid_metadata(struct iovec_wrapper *iovw, Context *context) { 361 (void) iovw_put_string_field_free(iovw, "COREDUMP_PROC_LIMITS=", t); 362 363 p = procfs_file_alloca(pid, "cgroup"); 364- if (read_full_virtual_file(p, &t, NULL) >=0) 365+ if (read_full_virtual_file(p, &t, NULL) >= 0) 366 (void) iovw_put_string_field_free(iovw, "COREDUMP_PROC_CGROUP=", t); 367 368 p = procfs_file_alloca(pid, "mountinfo"); 369- if (read_full_virtual_file(p, &t, NULL) >=0) 370+ if (read_full_virtual_file(p, &t, NULL) >= 0) 371 (void) iovw_put_string_field_free(iovw, "COREDUMP_PROC_MOUNTINFO=", t); 372 373+ /* We attach /proc/auxv here. ELF coredumps also contain a note for this (NT_AUXV), see elf(5). */ 374+ p = procfs_file_alloca(pid, "auxv"); 375+ if (read_full_virtual_file(p, &t, &size) >= 0) { 376+ char *buf = malloc(strlen("COREDUMP_PROC_AUXV=") + size + 1); 377+ if (buf) { 378+ /* Add a dummy terminator to make save_context() happy. */ 379+ *((uint8_t*) mempcpy(stpcpy(buf, "COREDUMP_PROC_AUXV="), t, size)) = '\0'; 380+ (void) iovw_consume(iovw, buf, size + strlen("COREDUMP_PROC_AUXV=")); 381+ } 382+ 383+ free(t); 384+ } 385+ 386 if (get_process_cwd(pid, &t) >= 0) 387 (void) iovw_put_string_field_free(iovw, "COREDUMP_CWD=", t); 388 389-- 3902.30.2 391 392