Merge branch 'boretrk/futils_mktmp'

4 files changed, 57 insertions, 19 deletions

diff --git a/src/futils.c b/src/futils.c
index 454ed79de..318e878f5 100644
--- a/src/futils.c
+++ b/src/futils.c
@@ -26,30 +26,32 @@ int git_futils_mkpath2file(const char *file_path, const mode_t mode)
 
 int git_futils_mktmp(git_str *path_out, const char *filename, mode_t mode)
 {
+	const int open_flags = O_RDWR | O_CREAT | O_EXCL | O_BINARY | O_CLOEXEC;
+	/* TMP_MAX is unrelated to mktemp but should provide a reasonable amount */
+	unsigned int tries = TMP_MAX;
 	int fd;
-	mode_t mask;
 
-	p_umask(mask = p_umask(0));
+	while (tries--) {
+		git_str_sets(path_out, filename);
+		git_str_puts(path_out, "_git2_XXXXXX");
 
-	git_str_sets(path_out, filename);
-	git_str_puts(path_out, "_git2_XXXXXX");
-
-	if (git_str_oom(path_out))
-		return -1;
+		if (git_str_oom(path_out))
+			return -1;
 
-	if ((fd = p_mkstemp(path_out->ptr)) < 0) {
-		git_error_set(GIT_ERROR_OS,
-			"failed to create temporary file '%s'", path_out->ptr);
-		return -1;
-	}
+		/* Using mktemp is safe when we open with O_CREAT | O_EXCL */
+		p_mktemp(path_out->ptr);
+		/* mktemp sets template to empty string on failure */
+		if (path_out->ptr[0] == '\0')
+			break;
 
-	if (p_chmod(path_out->ptr, (mode & ~mask))) {
-		git_error_set(GIT_ERROR_OS,
-			"failed to set permissions on file '%s'", path_out->ptr);
-		return -1;
+		if ((fd = p_open(path_out->ptr, open_flags, mode)) >= 0)
+			return fd;
 	}
 
-	return fd;
+	git_error_set(GIT_ERROR_OS,
+		"failed to create temporary file '%s'", path_out->ptr);
+	git_str_dispose(path_out);
+	return -1;
 }
 
 int git_futils_creat_withpath(const char *path, const mode_t dirmode, const mode_t mode)
diff --git a/src/futils.h b/src/futils.h
index f0d7ec3b0..1827cbafe 100644
--- a/src/futils.h
+++ b/src/futils.h
@@ -173,8 +173,16 @@ typedef enum {
 extern int git_futils_rmdir_r(const char *path, const char *base, uint32_t flags);
 
 /**
- * Create and open a temporary file with a `_git2_` suffix.
- * Writes the filename into path_out.
+ * Create and open a temporary file with a `_git2_` suffix in a
+ * protected directory; the file created will created will honor
+ * the current `umask`.  Writes the filename into path_out.
+ *
+ * This function is *NOT* suitable for use in temporary directories
+ * that are world writable.  It uses `mktemp` (for portability) and
+ * many `mktemp` implementations use weak random characters.  It
+ * should only be assumed to be suitable for atomically writing
+ * a new file in a directory that you control.
+ *
  * @return On success, an open file descriptor, else an error code < 0.
  */
 extern int git_futils_mktmp(git_str *path_out, const char *filename, mode_t mode);
diff --git a/src/posix.h b/src/posix.h
index d98bc82ca..1757764a2 100644
--- a/src/posix.h
+++ b/src/posix.h
@@ -9,6 +9,7 @@
 
 #include "common.h"
 
+#include <stdlib.h>
 #include <fcntl.h>
 #include <time.h>
 
@@ -130,6 +131,7 @@ extern ssize_t p_pwrite(int fd, const void *data, size_t size, off64_t offset);
 
 #define p_close(fd) close(fd)
 #define p_umask(m) umask(m)
+#define p_mktemp(p) mktemp(p)
 
 extern int p_open(const char *path, int flags, ...);
 extern int p_creat(const char *path, mode_t mode);
diff --git a/tests/core/futils.c b/tests/core/futils.c
index b87ea183b..3501765f6 100644
--- a/tests/core/futils.c
+++ b/tests/core/futils.c
@@ -87,3 +87,29 @@ void test_core_futils__recursive_rmdir_keeps_symlink_targets(void)
 	cl_must_pass(p_rmdir("dir-target"));
 	cl_must_pass(p_unlink("file-target"));
 }
+
+void test_core_futils__mktmp_umask(void)
+{
+#ifdef GIT_WIN32
+	cl_skip();
+#else
+	git_str path = GIT_STR_INIT;
+	struct stat st;
+	int fd;
+
+	umask(0);
+	cl_assert((fd = git_futils_mktmp(&path, "foo", 0777)) >= 0);
+	cl_must_pass(p_fstat(fd, &st));
+	cl_assert_equal_i(st.st_mode & 0777, 0777);
+	cl_must_pass(p_unlink(path.ptr));
+	close(fd);
+
+	umask(077);
+	cl_assert((fd = git_futils_mktmp(&path, "foo", 0777)) >= 0);
+	cl_must_pass(p_fstat(fd, &st));
+	cl_assert_equal_i(st.st_mode & 0777, 0700);
+	cl_must_pass(p_unlink(path.ptr));
+	close(fd);
+	git_str_dispose(&path);
+#endif
+}