From f00fed20092b6a42283f29c6ee1f58244d74b545 Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Thu, 6 Aug 2020 09:31:11 +0200 Subject: main: create new file when writing pidfile When writing the pidfile, open the file with the O_CREAT|O_EXCL flags to avoid following a symlink and writing the PID to an unexpected file, when chronyd still has the root privileges. The Linux open(2) man page warns about O_EXCL not working as expected on NFS versions before 3 and Linux versions before 2.6. Saving pidfiles on a distributed filesystem like NFS is not generally expected, but if there is a reason to do that, these old kernel and NFS versions are not considered to be supported for saving files by chronyd. This is a minimal backport specific to this issue of the following commits: - commit 2fc8edacb810 ("use PATH_MAX") - commit f4c6a00b2a11 ("logging: call exit() in LOG_Message()") - commit 7a4c396bba8f ("util: add functions for common file operations") - commit e18903a6b563 ("switch to new util file functions") Reported-by: Matthias Gerstner Upstream-Status: Backport [https://git.tuxfamily.org/chrony/chrony.git/commit/?id=f00fed20092b6a42283f29c6ee1f58244d74b545] CVE: CVE-2020-14367 Signed-off-by: Anatol Belski diff --git a/logging.c b/logging.c index d2296e0..fd7f900 100644 --- a/logging.c +++ b/logging.c @@ -171,6 +171,7 @@ void LOG_Message(LOG_Severity severity, system_log = 0; log_message(1, severity, buf); } + exit(1); break; default: assert(0); diff --git a/main.c b/main.c index 6ccf32e..8edb2e1 100644 --- a/main.c +++ b/main.c @@ -281,13 +281,9 @@ write_pidfile(void) if (!pidfile[0]) return; - out = fopen(pidfile, "w"); - if (!out) { - LOG_FATAL("Could not open %s : %s", pidfile, strerror(errno)); - } else { - fprintf(out, "%d\n", (int)getpid()); - fclose(out); - } + out = UTI_OpenFile(NULL, pidfile, NULL, 'W', 0644); + fprintf(out, "%d\n", (int)getpid()); + fclose(out); } /* ================================================== */ diff --git a/sysincl.h b/sysincl.h index 296c5e6..873a3bd 100644 --- a/sysincl.h +++ b/sysincl.h @@ -37,6 +37,7 @@ #include #include #include +#include #include #include #include diff --git a/util.c b/util.c index e7e3442..83b3b20 100644 --- a/util.c +++ b/util.c @@ -1179,6 +1179,101 @@ UTI_CheckDirPermissions(const char *path, mode_t perm, uid_t uid, gid_t gid) /* ================================================== */ +static int +join_path(const char *basedir, const char *name, const char *suffix, + char *buffer, size_t length, LOG_Severity severity) +{ + const char *sep; + + if (!basedir) { + basedir = ""; + sep = ""; + } else { + sep = "/"; + } + + if (!suffix) + suffix = ""; + + if (snprintf(buffer, length, "%s%s%s%s", basedir, sep, name, suffix) >= length) { + LOG(severity, "File path %s%s%s%s too long", basedir, sep, name, suffix); + return 0; + } + + return 1; +} + +/* ================================================== */ + +FILE * +UTI_OpenFile(const char *basedir, const char *name, const char *suffix, + char mode, mode_t perm) +{ + const char *file_mode; + char path[PATH_MAX]; + LOG_Severity severity; + int fd, flags; + FILE *file; + + severity = mode >= 'A' && mode <= 'Z' ? LOGS_FATAL : LOGS_ERR; + + if (!join_path(basedir, name, suffix, path, sizeof (path), severity)) + return NULL; + + switch (mode) { + case 'r': + case 'R': + flags = O_RDONLY; + file_mode = "r"; + if (severity != LOGS_FATAL) + severity = LOGS_DEBUG; + break; + case 'w': + case 'W': + flags = O_WRONLY | O_CREAT | O_EXCL; + file_mode = "w"; + break; + case 'a': + case 'A': + flags = O_WRONLY | O_CREAT | O_APPEND; + file_mode = "a"; + break; + default: + assert(0); + return NULL; + } + +try_again: + fd = open(path, flags, perm); + if (fd < 0) { + if (errno == EEXIST) { + if (unlink(path) < 0) { + LOG(severity, "Could not remove %s : %s", path, strerror(errno)); + return NULL; + } + DEBUG_LOG("Removed %s", path); + goto try_again; + } + LOG(severity, "Could not open %s : %s", path, strerror(errno)); + return NULL; + } + + UTI_FdSetCloexec(fd); + + file = fdopen(fd, file_mode); + if (!file) { + LOG(severity, "Could not open %s : %s", path, strerror(errno)); + close(fd); + return NULL; + } + + DEBUG_LOG("Opened %s fd=%d mode=%c", path, fd, mode); + + return file; +} + +/* ================================================== */ + void UTI_DropRoot(uid_t uid, gid_t gid) { diff --git a/util.h b/util.h index e3d6767..a2481cc 100644 --- a/util.h +++ b/util.h @@ -176,6 +176,17 @@ extern int UTI_CreateDirAndParents(const char *path, mode_t mode, uid_t uid, gid permissions and its uid/gid must match the specified values. */ extern int UTI_CheckDirPermissions(const char *path, mode_t perm, uid_t uid, gid_t gid); +/* Open a file. The full path of the file is constructed from the basedir + (may be NULL), '/' (if basedir is not NULL), name, and suffix (may be NULL). + Created files have specified permissions (umasked). Returns NULL on error. + The following modes are supported (if the mode is an uppercase character, + errors are fatal): + r/R - open an existing file for reading + w/W - open a new file for writing (remove existing file) + a/A - open an existing file for appending (create if does not exist) */ +extern FILE *UTI_OpenFile(const char *basedir, const char *name, const char *suffix, + char mode, mode_t perm); + /* Set process user/group IDs and drop supplementary groups */ extern void UTI_DropRoot(uid_t uid, gid_t gid); -- cgit v0.10.2