Further fix for CVE-2020-28935, so the chown is omitted when the pidfile

fails due to a symlink.

diff --git a/daemon/unbound.c b/daemon/unbound.c
index 45537d5c9ed99ee1c98bbb2fa09ffa9f85d959a8..0efa353da59e9ebb9c3460c42868c5885ff39ca9 100644 (file)
--- a/daemon/unbound.c
+++ b/daemon/unbound.c
@@ -337,8 +337,9 @@ readpid (const char* file)
 /** write pid to file. 
  * @param pidfile: file name of pid file.
  * @param pid: pid to write to file.
+ * @return false on failure
  */
-static void
+static int
 writepid (const char* pidfile, pid_t pid)
 {
        int fd;
@@ -353,7 +354,7 @@ writepid (const char* pidfile, pid_t pid)
                , 0644)) == -1) {
                log_err("cannot open pidfile %s: %s", 
                        pidfile, strerror(errno));
-               return;
+               return 0;
        }
        while(count < strlen(pidbuf)) {
                ssize_t r = write(fd, pidbuf+count, strlen(pidbuf)-count);
@@ -362,11 +363,13 @@ writepid (const char* pidfile, pid_t pid)
                                continue;
                        log_err("cannot write to pidfile %s: %s",
                                pidfile, strerror(errno));
-                       break;
+                       close(fd);
+                       return 0;
                }
                count += r;
        }
        close(fd);
+       return 1;
 }
 
 /**
@@ -520,16 +523,17 @@ perform_setup(struct daemon* daemon, struct config_file* cfg, int debug_mode,
        /* write new pidfile (while still root, so can be outside chroot) */
 #ifdef HAVE_KILL
        if(cfg->pidfile && cfg->pidfile[0] && need_pidfile) {
-               writepid(daemon->pidfile, getpid());
-               if(cfg->username && cfg->username[0] && cfg_uid != (uid_t)-1 &&
-                       pidinchroot) {
+               if(writepid(daemon->pidfile, getpid())) {
+                       if(cfg->username && cfg->username[0] && cfg_uid != (uid_t)-1 &&
+                               pidinchroot) {
 #  ifdef HAVE_CHOWN
-                       if(chown(daemon->pidfile, cfg_uid, cfg_gid) == -1) {
-                               verbose(VERB_QUERY, "cannot chown %u.%u %s: %s",
-                                       (unsigned)cfg_uid, (unsigned)cfg_gid,
-                                       daemon->pidfile, strerror(errno));
-                       }
+                               if(chown(daemon->pidfile, cfg_uid, cfg_gid) == -1) {
+                                       verbose(VERB_QUERY, "cannot chown %u.%u %s: %s",
+                                               (unsigned)cfg_uid, (unsigned)cfg_gid,
+                                               daemon->pidfile, strerror(errno));
+                               }
 #  endif /* HAVE_CHOWN */
+                       }
                }
        }
 #else