From: W.C.A. Wijngaards <wouter@nlnetlabs.nl>
Date: Mon, 23 Nov 2020 12:48:04 +0000 (+0100)
Subject: Further fix for CVE-2020-28935, so the chown is omitted when the pidfile
X-Git-Tag: release-1.13.0rc1~9
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=19f8f4d9f99a44906ab9dcc46d44da299fde3506;p=thirdparty%2Funbound.git

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 45537d5c9..0efa353da 100644
--- 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