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 <tobias@stoeckmann.org>

1 files 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);
 			}