From bec4e4373208d02927cb6cf83de9ba0da3df4545 Mon Sep 17 00:00:00 2001 From: Gabriel Kihlman Date: Wed, 15 May 2019 12:38:24 +0200 Subject: [PATCH] Fix a TOCTOU case when opening the debug_file This also unifies the code between pam-u2f and yubico-pam which means removing the O_CREAT flag here. This is however in line with the README and the behaviour of pam-u2f. Also, the previous code did lstat on the path before and if it did not exist it would not have moved on to the open() anyways. --- pam_yubico.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/pam_yubico.c b/pam_yubico.c index f8ba801..0f7ded1 100644 --- a/pam_yubico.c +++ b/pam_yubico.c @@ -790,6 +790,9 @@ restpriv_out: static void parse_cfg (int flags, int argc, const char **argv, struct cfg *cfg) { + struct stat st; + FILE *file = NULL; + int fd = -1; int i; memset (cfg, 0, sizeof(struct cfg)); @@ -879,24 +882,15 @@ parse_cfg (int flags, int argc, const char **argv, struct cfg *cfg) } else { - struct stat st; - int fd; - FILE *file; - if(lstat(filename, &st) == 0) + fd = open(filename, O_WRONLY | O_APPEND | O_CLOEXEC | O_NOFOLLOW | O_NOCTTY); + if (fd >= 0 && (fstat(fd, &st) == 0) && S_ISREG(st.st_mode)) { - if(S_ISREG(st.st_mode)) + file = fdopen(fd, "a"); + if(file != NULL) { - fd = open(filename, O_WRONLY | O_CREAT | O_APPEND | O_CLOEXEC, S_IRUSR | S_IWUSR | S_IRGRP); - if (fd >= 0) - { - file = fdopen(fd, "a"); - if (file) - { - cfg->debug_file = file; - } else { - close(fd); - } - } + cfg->debug_file = file; + file = NULL; + fd = -1; } } } @@ -940,6 +934,12 @@ parse_cfg (int flags, int argc, const char **argv, struct cfg *cfg) DBG ("token_id_length=%u", cfg->token_id_length); DBG ("mode=%s", cfg->mode == CLIENT ? "client" : "chresp" ); DBG ("chalresp_path=%s", cfg->chalresp_path ? cfg->chalresp_path : "(null)"); + + if (fd != -1) + close(fd); + + if (file != NULL) + fclose(file); } PAM_EXTERN int