From 945a580d3066c5691f8f6c13f41135946440ba0c Mon Sep 17 00:00:00 2001 From: "Robin H. Johnson" Date: Thu, 2 Jan 2025 09:54:32 -0800 Subject: [PATCH] fix: improve file checks for procfs knobs CodeQL checks flagged this pattern as a TOCTOU race. Signed-off-by: Robin H. Johnson --- privsep-linux.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/privsep-linux.c b/privsep-linux.c index c80269e..63e2b11 100644 --- a/privsep-linux.c +++ b/privsep-linux.c @@ -183,25 +183,32 @@ static int set_interface_var(const char *iface, const char *var, const char *nam /* No path traversal */ if (!iface[0] || !strcmp(iface, ".") || !strcmp(iface, "..") || strchr(iface, '/')) - goto cleanup; + goto errmsg; + /* If the file does NOT exist, do NOT create it; this is not an error, as we have to support old & new names */ if (access(spath, F_OK) != 0) goto cleanup; - fp = fopen(spath, "w"); - if (!fp) { - if (name) - flog(LOG_ERR, "failed to set %s (%u) for %s: %s", name, val, iface, strerror(errno)); - goto cleanup; - } + // recheck to avoid TOCTOU anyway + int fd = open(spath, O_WRONLY, S_IRWXU); + if (fd == -1) + goto errmsg; + + fp = fdopen(fd, "w"); + if (!fp) + goto errmsg; if (0 > fprintf(fp, "%u", val)) { - goto cleanup; + goto errmsg; } retval = 0; +errmsg: + if (name && retval != 0) + flog(LOG_ERR, "failed to set %s (%u) for %s: %s", name, val, iface, strerror(errno)); cleanup: + if (fp) fclose(fp);