From: benno <benno@openbsd.org>
Date: Sun, 29 Jul 2018 20:15:23 +0000 (+0000)
Subject: replace hand-rolled tmp files with mkstemp()
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=4716c154ab62a95af1d3b2bdbd6a4bb6ca7ca157;p=openbsd

replace hand-rolled tmp files with mkstemp()
ok florian@ back in april, reminded by theo.
---

diff --git a/usr.sbin/acme-client/fileproc.c b/usr.sbin/acme-client/fileproc.c
index cd4298e62a4..00ce339670a 100644
--- a/usr.sbin/acme-client/fileproc.c
+++ b/usr.sbin/acme-client/fileproc.c
@@ -1,4 +1,4 @@
-/*	$Id: fileproc.c,v 1.14 2017/01/24 13:32:55 jsing Exp $ */
+/*	$Id: fileproc.c,v 1.15 2018/07/29 20:15:23 benno Exp $ */
 /*
  * Copyright (c) 2016 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -15,6 +15,8 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+#include <sys/stat.h>
+
 #include <err.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -27,37 +29,51 @@
 #include "extern.h"
 
 static int
-serialise(const char *tmp, const char *real,
-    const char *v, size_t vsz, const char *v2, size_t v2sz)
+serialise(const char *real, const char *v, size_t vsz, const char *v2, size_t v2sz)
 {
-	int	 fd;
+	int	  fd;
+	char	 *tmp;
 
 	/*
 	 * Write into backup location, overwriting.
-	 * Then atomically (?) do the rename.
+	 * Then atomically do the rename.
 	 */
 
-	fd = open(tmp, O_WRONLY|O_CREAT|O_TRUNC, 0444);
-	if (fd == -1) {
-		warn("%s", tmp);
-		return 0;
-	} else if ((ssize_t)vsz != write(fd, v, vsz)) {
-		warnx("%s", tmp);
-		close(fd);
-		return 0;
-	} else if (v2 != NULL && write(fd, v2, v2sz) != (ssize_t)v2sz) {
-		warnx("%s", tmp);
-		close(fd);
-		return 0;
-	} else if (close(fd) == -1) {
-		warn("%s", tmp);
+	if (asprintf(&tmp, "%s.XXXXXXXXXX", real) == -1) {
+		warn("asprintf");
 		return 0;
-	} else if (rename(tmp, real) == -1) {
+	}
+	if ((fd = mkstemp(tmp)) == -1) {
+		warn("mkstemp");
+		goto out;
+	}
+	if (fchmod(fd, 0444) == -1) {
+		warn("fchmod");
+		goto out;
+	}
+	if ((ssize_t)vsz != write(fd, v, vsz)) {
+		warnx("write");
+		goto out;
+	}
+	if (v2 != NULL && write(fd, v2, v2sz) != (ssize_t)v2sz) {
+		warnx("write");
+		goto out;
+	}
+	if (close(fd) == -1)
+		goto out;
+	if (rename(tmp, real) == -1) {
 		warn("%s", real);
-		return 0;
+		goto out;
 	}
 
+	free(tmp);
 	return 1;
+out:
+	if (fd != -1)
+		close(fd);
+	(void) unlink(tmp);
+	free(tmp);
+	return 0;
 }
 
 int
@@ -65,15 +81,11 @@ fileproc(int certsock, const char *certdir, const char *certfile, const char
     *chainfile, const char *fullchainfile)
 {
 	char		*csr = NULL, *ch = NULL;
-	char		*certfile_bak = NULL, *chainfile_bak = NULL;
-	char		*fullchainfile_bak = NULL;
 	size_t		 chsz, csz;
 	int		 rc = 0;
 	long		 lval;
 	enum fileop	 op;
 
-	/* File-system and sandbox jailing. */
-
 	if (chroot(certdir) == -1) {
 		warn("chroot");
 		goto out;
@@ -85,9 +97,9 @@ fileproc(int certsock, const char *certdir, const char *certfile, const char
 
 	/*
 	 * rpath and cpath for rename, wpath and cpath for
-	 * writing to the temporary.
+	 * writing to the temporary. fattr for fchmod.
 	 */
-	if (pledge("stdio cpath wpath rpath", NULL) == -1) {
+	if (pledge("stdio cpath wpath rpath fattr", NULL) == -1) {
 		warn("pledge");
 		goto out;
 	}
@@ -148,32 +160,12 @@ fileproc(int certsock, const char *certdir, const char *certfile, const char
 	 * Start by downloading the chain PEM as a buffer.
 	 * This is not NUL-terminated, but we're just going to guess
 	 * that it's well-formed and not actually touch the data.
-	 * Once downloaded, dump it into CHAIN_BAK.
 	 */
-
-	if (certfile)
-		if (asprintf(&certfile_bak, "%s~", certfile) == -1) {
-			warn("asprintf");
-			goto out;
-		}
-
-	if (chainfile)
-		if (asprintf(&chainfile_bak, "%s~", chainfile) == -1) {
-			warn("asprintf");
-			goto out;
-		}
-
-	if (fullchainfile)
-		if (asprintf(&fullchainfile_bak, "%s~", fullchainfile) == -1) {
-			warn("asprintf");
-			goto out;
-		}
-
 	if ((ch = readbuf(certsock, COMM_CHAIN, &chsz)) == NULL)
 		goto out;
 
 	if (chainfile) {
-		if (!serialise(chainfile_bak, chainfile, ch, chsz, NULL, 0))
+		if (!serialise(chainfile, ch, chsz, NULL, 0))
 			goto out;
 
 		dodbg("%s/%s: created", certdir, chainfile);
@@ -190,7 +182,7 @@ fileproc(int certsock, const char *certdir, const char *certfile, const char
 		goto out;
 
 	if (certfile) {
-		if (!serialise(certfile_bak, certfile, csr, csz, NULL, 0))
+		if (!serialise(certfile, csr, csz, NULL, 0))
 			goto out;
 
 		dodbg("%s/%s: created", certdir, certfile);
@@ -203,7 +195,7 @@ fileproc(int certsock, const char *certdir, const char *certfile, const char
 	 * on-file certificates were changed.
 	 */
 	if (fullchainfile) {
-		if (!serialise(fullchainfile_bak, fullchainfile, csr, csz, ch,
+		if (!serialise(fullchainfile, csr, csz, ch,
 		    chsz))
 			goto out;
 
@@ -215,8 +207,5 @@ out:
 	close(certsock);
 	free(csr);
 	free(ch);
-	free(certfile_bak);
-	free(chainfile_bak);
-	free(fullchainfile_bak);
 	return rc;
 }