From 2c38c9da3ea45d6c692314ad2e2f7213de2afaeb Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Fri, 12 Jul 2019 19:43:59 +0200 Subject: Check stat for error before setting permissions. When setting wallpapers without --no-fehbg option, a ~/.fehbg file is created. This file is set to be an executable for later re-use. Calling stat() without checking the return value can lead to issues. If the call fails, then s.st_mode is undefined and excessive permissions could be set to .fehbg, at worst even setuid/setgid bits for a world writable file. While adjusting this, I changed the code to use fstat() and fchmod() to avoid a further -- but very unlikely -- issue: race condition in form of TOCTOU. If the file ~/.fehsetbg is replaced by a symlink right before the chmod call, then a different file would be set executable + the default mode of the (newly created) file. I don't expect this to be a real world issue but changed this part "while at it" anyway for more robust code and a good example on how to handle files. Signed-off-by: Tobias Stoeckmann --- src/wallpaper.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/wallpaper.c b/src/wallpaper.c index 9df259f..96b4f90 100644 --- a/src/wallpaper.c +++ b/src/wallpaper.c @@ -452,6 +452,7 @@ void feh_wm_set_bg(char *fil, Imlib_Image im, int centered, int scaled, home = getenv("HOME"); if (home) { FILE *fp; + int fd; char *path; char *absolute_path; struct stat s; @@ -519,11 +520,11 @@ void feh_wm_set_bg(char *fil, Imlib_Image im, int centered, int scaled, free(absolute_path); } fputc('\n', fp); - fclose(fp); - stat(path, &s); - if (chmod(path, s.st_mode | S_IXUSR | S_IXGRP) != 0) { + fd = fileno(fp); + if (fstat(fd, &s) != 0 || fchmod(fd, s.st_mode | S_IXUSR | S_IXGRP) != 0) { weprintf("Can't set %s as executable", path); } + fclose(fp); } free(path); } -- cgit v1.2.3