Merge pull request #1236 from kernelkit/recopy

bin: copy: Fix various resource leaks introduced by refactor

Author: wkz

board/common/rootfs/usr/libexec/infix/mksshkey

@@ -18,6 +18,7 @@ echo -e "$PUBLIC" >> "$TMP"
 echo -e "-----END RSA PUBLIC KEY-----" >> "$TMP"
 
 ssh-keygen -i -m PKCS8 -f "$TMP" > "$DIR/$NAME.pub"
+rm "$TMP"
 chmod 0600 "$DIR/$NAME.pub"
 chmod 0600 "$DIR/$NAME"
 chown sshd:sshd "$DIR/$NAME.pub"

src/bin/copy.c

@@ -155,6 +155,30 @@ static bool is_uri(const char *str)
 	return strstr(str, "://") != NULL;
 }
 
+static char *mktmp(void)
+{
+	mode_t oldmask;
+	char *path;
+	int fd;
+
+	path = strdup("/tmp/copy-XXXXXX");
+	if (!path)
+		goto err;
+
+	oldmask = umask(0077);
+	fd = mkstemp(path);
+	umask(oldmask);
+
+	if (fd < 0)
+		goto err;
+
+	close(fd);
+	return path;
+err:
+	free(path);
+	return NULL;
+}
+
 static void rmtmp(const char *path)
 {
 	if (remove(path)) {
@@ -338,7 +362,7 @@ static int curl(char *op, const char *path, const char *uri)
 	char *argv[] =  {
 		"curl", "-L", op, NULL, NULL, NULL, NULL, NULL,
 	};
-	int err;
+	int err = 1;
 
 	argv[3] = strdup(path);
 	argv[4] = strdup(uri);
@@ -390,7 +414,7 @@ static int cp(const char *srcpath, const char *dstpath)
 	char *argv[] =  {
 		"cp", NULL, NULL, NULL,
 	};
-	int err;
+	int err = 1;
 
 	argv[1] = strdup(srcpath);
 	argv[2] = strdup(dstpath);
@@ -445,7 +469,7 @@ static int resolve_src(const char **src, const struct infix_ds **ds, char **path
 	*src = infix_ds(*src, ds);
 
 	if (*ds || is_uri(*src)) {
-		*path = tempnam(NULL, NULL);
+		*path = mktmp();
 		if (!*path)
 			return 1;
 
@@ -477,7 +501,7 @@ static int resolve_dst(const char **dst, const struct infix_ds **ds, char **path
 		if (!(*ds)->path)
 			return 0;
 
-		*path = (*ds)->path;
+		*path = strdup((*ds)->path);
 	} else if (is_uri(*dst)) {
 		return 0;
 	} else {
@@ -499,8 +523,8 @@ static int resolve_dst(const char **dst, const struct infix_ds **ds, char **path
 
 static int copy(const char *src, const char *dst)
 {
+	char *srcpath = NULL, *dstpath = NULL;
 	const struct infix_ds *srcds, *dstds;
-	char *srcpath, *dstpath;
 	bool rmsrc = false;
 	mode_t oldmask;
 	int err = 1;
@@ -534,6 +558,9 @@ static int copy(const char *src, const char *dst)
 	if (rmsrc)
 		rmtmp(srcpath);
 
+	free(dstpath);
+	free(srcpath);
+
 	sync();
 	umask(oldmask);
 	return err;

src/bin/erase.c

@@ -16,7 +16,7 @@ static int sanitize;
 static int do_erase(const char *name)
 {
 	char *path;
-	int rc;
+	int rc = 0;
 
 	path = cfg_adjust(name, NULL, sanitize);
 	if (!path) {
@@ -25,10 +25,8 @@ static int do_erase(const char *name)
 		goto out;
 	}
 
-	if (!yorn("Remove %s, are you sure?", path)) {
-		rc = 0;
+	if (!yorn("Remove %s, are you sure?", path))
 		goto out;
-	}
 
 	if (remove(path)) {
 		fprintf(stderr, ERRMSG "failed removing %s: %s\n", path, strerror(errno));

src/bin/util.c

@@ -138,16 +138,26 @@ char *cfg_adjust(const char *path, const char *template, bool sanitize)
 		     strchr(basename, '.') ? "" : ".cfg") < 0)
 		goto err;
 
-	/* If file exists, resolve symlinks and verify still in whitelist */
-	if (sanitize && !access(expanded, F_OK)) {
+	if (sanitize) {
 		resolved = realpath(expanded, NULL);
-		if (!resolved || !path_allowed(resolved))
+		if (!resolved) {
+			if (errno == ENOENT)
+				goto out;
+			else
+				goto err;
+		}
+
+		/* File exists, make sure that the resolved symlink
+		 * still matches the whitelist.
+		 */
+		if (!path_allowed(resolved))
 			goto err;
 
 		free(expanded);
 		expanded = resolved;
 	}
 
+out:
 	return expanded;
 
 err: