From: Vincent Bernat <bernat@luffy.cx>
Date: Fri, 22 Nov 2013 22:03:51 +0000 (+0100)
Subject: coverity: fix TOCTOU problem when creating chroot
X-Git-Tag: 0.7.8~80
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=b7e2b07c1eb6e32ca2f2aa95bb60250fa90c80c5;p=thirdparty%2Flldpd.git

coverity: fix TOCTOU problem when creating chroot

It is believed that checking for existence of a directory or a file
before an action is useless since it could be created after the check
and before the action. Therefore, it is better to just try to do the
action and handle any failure gracefully.

When setting up the chroot, instead of checking if it already exists, we
create it and don't display an error if it is already set up.
---

diff --git a/src/daemon/priv.c b/src/daemon/priv.c
index c8bce823..9c633d65 100644
--- a/src/daemon/priv.c
+++ b/src/daemon/priv.c
@@ -440,48 +440,45 @@ static void
 priv_setup_chroot(const char *chrootdir)
 {
 	/* Create chroot if it does not exist */
-	struct stat schroot;
-	if (stat(chrootdir, &schroot) == -1) {
-		if (errno != ENOENT)
-			fatal("privsep", "chroot directory does not exist");
-		if (mkdir(chrootdir, 0755) == -1)
+	if (mkdir(chrootdir, 0755) == -1) {
+		if (errno != EEXIST)
 			fatal("privsep", "unable to create chroot directory");
+	} else {
 		log_info("privsep", "created chroot directory %s",
 		    chrootdir);
 	}
 
 	/* Check if /etc/localtime exists in chroot or outside chroot */
 	char path[1024];
+	int source = -1, destination = -1;
 	if (snprintf(path, sizeof(path),
 		"%s" LOCALTIME, chrootdir) >= sizeof(path))
 		return;
-	if (stat(path, &schroot) != -1 ||
-	    stat(LOCALTIME, &schroot) == -1) return;
+	if ((source = open(LOCALTIME, O_RDONLY)) == -1) {
+		if (errno == ENOENT)
+			return;
+	}
 
 	/* Prepare copy of /etc/localtime */
 	path[strlen(chrootdir) + 4] = '\0';
-	if (stat(path, &schroot) == -1) {
-		if (errno != ENOENT) return;
-		if (mkdir(path, 0755) == -1) {
+	if (mkdir(path, 0755) == -1) {
+		if (errno != EEXIST) {
 			log_warn("privsep", "unable to create %s directory",
 			    path);
+			close(source);
 			return;
 		}
 	}
 	path[strlen(chrootdir) + 4] = '/';
 
 	/* Do copy */
-	int source = -1, destination = -1;
 	char buffer[1024];
 	ssize_t n;
-	if ((source = open(LOCALTIME, O_RDONLY)) == -1) {
-		log_warn("privsep", "cannot read " LOCALTIME);
-		return;
-	}
 	mode_t old = umask(S_IWGRP | S_IWOTH);
 	if ((destination = open(path,
-		    O_WRONLY | O_CREAT | O_TRUNC, 0666)) == -1) {
-		log_warn("privsep", "cannot create %s", path);
+		    O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, 0666)) == -1) {
+		if (errno != EEXIST)
+			log_warn("privsep", "cannot create %s", path);
 		close(source);
 		umask(old);
 		return;