1 files changed, 34 insertions, 34 deletions

diff --git a/util/libreboot-utils/lib/mkhtemp.c b/util/libreboot-utils/lib/mkhtemp.c
index 2726cb02..61479b25 100644
--- a/util/libreboot-utils/lib/mkhtemp.c
+++ b/util/libreboot-utils/lib/mkhtemp.c
@@ -206,8 +206,8 @@ env_tmpdir(int bypass_all_sticky_checks, char **tmpdir,
 	int allow_noworld_unsticky;
 	int saved_errno = errno;
 
-	char tmp[] = "/tmp";
-	char vartmp[] = "/var/tmp";
+	static const char tmp[] = "/tmp";
+	static const char vartmp[] = "/var/tmp";
 
 	/* tmpdir is a user override, if set */
 	if (override_tmpdir == NULL)
@@ -238,26 +238,26 @@ env_tmpdir(int bypass_all_sticky_checks, char **tmpdir,
 
 	allow_noworld_unsticky = 0;
 
-	if (world_writeable_and_sticky("/tmp",
+	if (world_writeable_and_sticky(tmp,
 	    allow_noworld_unsticky,
 	    bypass_all_sticky_checks)) {
 
 		if (tmpdir != NULL)
-			*tmpdir = tmp;
+			*tmpdir = (char *)tmp;
 
 		errno = saved_errno;
-		return "/tmp";
+		return (char *)tmp;
 	}
 
-	if (world_writeable_and_sticky("/var/tmp",
+	if (world_writeable_and_sticky(vartmp,
 	    allow_noworld_unsticky,
 	    bypass_all_sticky_checks)) {
 
 		if (tmpdir != NULL)
-			*tmpdir = vartmp;
+			*tmpdir = (char *)vartmp;
 
 		errno = saved_errno;
-		return "/var/tmp";
+		return (char *)vartmp;
 	}
 
 	return NULL;
@@ -295,11 +295,7 @@ tmpdir_policy(const char *path,
 	return 0;
 
 err_tmpdir_policy:
-
-	if (errno == saved_errno)
-		errno = EIO;
-
-	return -1;
+	return set_errno(saved_errno, EIO);
 }
 
 int
@@ -331,11 +327,11 @@ same_dir(const char *a, const char *b)
 	if (rval_scmp == 0)
 		goto success_same_dir;
 
-	fd_a = fs_open(a, O_RDONLY | O_DIRECTORY);
+	fd_a = fs_open(a, O_RDONLY | O_DIRECTORY | O_NOFOLLOW);
 	if (fd_a < 0)
 		goto err_same_dir;
 
-	fd_b = fs_open(b, O_RDONLY | O_DIRECTORY);
+	fd_b = fs_open(b, O_RDONLY | O_DIRECTORY | O_NOFOLLOW);
 	if (fd_b < 0)
 		goto err_same_dir;
 
@@ -377,10 +373,7 @@ err_same_dir:
 	close_on_eintr(&fd_a);
 	close_on_eintr(&fd_b);
 
-	if (errno == saved_errno)
-		errno = EIO;
-
-	return -1;
+	return set_errno(saved_errno, EIO);
 }
 
 /* bypass_all_sticky_checks: if set,
@@ -467,19 +460,15 @@ world_writeable_and_sticky(
 	goto sticky_hell; /* heaven visa denied */
 
 sticky_heaven:
-
 	close_on_eintr(&dirfd);
 	errno = saved_errno;
 
 	return 1;
 
 sticky_hell:
-
-	if (errno == saved_errno)
-		errno = EPERM;
-
 	close_on_eintr(&dirfd);
 
+	(void) set_errno(saved_errno, EPERM);
 	return 0;
 }
 
@@ -524,6 +513,17 @@ sticky_hell:
  * file ownership under hardened mode
  * (only lets you touch your own files/dirs)
  */
+/*
+ TODO:
+	some variables e.g. template vs suffix,
+	assumes they match.
+	we should test this explicitly,
+	but the way this is called is
+	currently safe - this would however
+	be nice for future library use
+	by outside projects.
+	this whole code needs to be reorganised
+*/
 int
 mkhtemp(int *fd,
     struct stat *st,
@@ -832,6 +832,12 @@ err:
 }
 #endif
 
+/* TODO: potential infinite loop under entropy failure.
+ * e.g. keeps returning low quality RNG, or atacker
+ * has control (DoS attack potential).
+ * possible solution: add a timeout (and abort if
+ * the timeout is reached)
+ */
 int
 mkhtemp_fill_random(char *p, size_t xc)
 {
@@ -911,6 +917,8 @@ int secure_file(int *fd,
 		if (lock_file(*fd, flags) == -1)
 			goto err_demons;
 
+		/* TODO: why would this be NULL? audit
+		 * to find out. we should always verify! */
 		if (expected != NULL)
 			if (fd_verify_identity(*fd, expected, &st_now) < 0)
 				goto err_demons;
@@ -923,11 +931,7 @@ int secure_file(int *fd,
 	return 0;
 
 err_demons:
-
-	if (errno == saved_errno)
-		errno = EIO;
-
-	return -1;
+	return set_errno(saved_errno, EIO);
 }
 
 int
@@ -1037,9 +1041,5 @@ lock_file(int fd, int flags)
 	return 0;
 
 err_lock_file:
-
-	if (errno == saved_errno)
-		errno = EIO;
-
-	return -1;
+	return set_errno(saved_errno, EIO);
 }