Merge branch 'boretrk/futils_mktmp'
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131
diff --git a/src/futils.c b/src/futils.c
index 454ed79..318e878 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 f0d7ec3..1827cba 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 d98bc82..1757764 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 b87ea18..3501765 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
+}