From 22892fb94b7da0018802363637928d21b7f00687 Mon Sep 17 00:00:00 2001 From: Kamil Dudka Date: Mon, 30 Apr 2018 18:46:13 +0200 Subject: [PATCH 14/39] do_mkdir: fix time-of-check/time-of-use race condition reason: fix time-of-check/time-of-use race condition https://github.com/logrotate/logrotate/pull/196 ... detected by Coverity Analysis Error: TOCTOU: config.c:362: fs_check_call: Calling function "stat" to perform check on "path". config.c:363: toctou: Calling function "mkdir" that uses "path" after a check function. This can cause a time-of-check, time-of-use race condition. Closes #196 --- config.c | 49 ++++++++++++++++++++++++++----------------------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/config.c b/config.c index 9cc66ac..4162cca 100644 --- a/config.c +++ b/config.c @@ -357,33 +357,36 @@ static char *readAddress(const char *configFile, int lineNum, const char *key, } static int do_mkdir(const char *path, mode_t mode, uid_t uid, gid_t gid) { - struct stat sb; - - if (stat(path, &sb) != 0) { - if (mkdir(path, mode) != 0 && errno != EEXIST) { - message(MESS_ERROR, "error creating %s: %s\n", - path, strerror(errno)); - return -1; - } - if (chown(path, uid, gid) != 0) { - message(MESS_ERROR, "error setting owner of %s to uid %d and gid %d: %s\n", - path, uid, gid, strerror(errno)); - return -1; - } - if (chmod(path, mode) != 0) { - message(MESS_ERROR, "error setting permissions of %s to 0%o: %s\n", - path, mode, strerror(errno)); - return -1; - } + if (mkdir(path, mode) == 0) { + /* newly created directory, set the owner and permissions */ + if (chown(path, uid, gid) != 0) { + message(MESS_ERROR, "error setting owner of %s to uid %d and gid %d: %s\n", + path, uid, gid, strerror(errno)); + return -1; } - else if (!S_ISDIR(sb.st_mode)) { - message(MESS_ERROR, "path %s already exists, but it is not a directory\n", - path); - errno = ENOTDIR; - return -1; + + if (chmod(path, mode) != 0) { + message(MESS_ERROR, "error setting permissions of %s to 0%o: %s\n", + path, mode, strerror(errno)); + return -1; } return 0; + } + + if (errno == EEXIST) { + /* path already exists, check whether it is a directory or not */ + struct stat sb; + if ((stat(path, &sb) == 0) && S_ISDIR(sb.st_mode)) + return 0; + + message(MESS_ERROR, "path %s already exists, but it is not a directory\n", path); + errno = ENOTDIR; + return -1; + } + + message(MESS_ERROR, "error creating %s: %s\n", path, strerror(errno)); + return -1; } static int mkpath(const char *path, mode_t mode, uid_t uid, gid_t gid) { -- 1.8.3.1