Skip to content

Commit 47efda9

Browse files
bk2204gitster
authored andcommitted
wrapper: use a CSPRNG to generate random file names
The current way we generate random file names is by taking the seconds and microseconds, plus the PID, and mixing them together, then encoding them. If this fails, we increment the value by 7777, and try again up to TMP_MAX times. Unfortunately, this is not the best idea from a security perspective. If we're writing into TMPDIR, an attacker can guess these values easily and prevent us from creating any temporary files at all by creating them all first. Even though we set TMP_MAX to 16384, this may be achievable in some contexts, even if unlikely to occur in practice. Fortunately, we can simply solve this by using the system cryptographically secure pseudorandom number generator (CSPRNG) to generate a random 64-bit value, and use that as before. Note that there is still a small bias here, but because a six-character sequence chosen out of 62 characters provides about 36 bits of entropy, the bias here is less than 2^-28, which is acceptable, especially considering we'll retry several times. Note that the use of a CSPRNG in generating temporary file names is also used in many libcs. glibc recently changed from an approach similar to ours to using a CSPRNG, and FreeBSD and OpenBSD also use a CSPRNG in this case. Even if the likelihood of an attack is low, we should still be at least as responsible in creating temporary files as libc is. Signed-off-by: brian m. carlson <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 05cd988 commit 47efda9

File tree

1 file changed

+4
-11
lines changed

1 file changed

+4
-11
lines changed

wrapper.c

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -463,8 +463,6 @@ int git_mkstemps_mode(char *pattern, int suffix_len, int mode)
463463
static const int num_letters = ARRAY_SIZE(letters) - 1;
464464
static const char x_pattern[] = "XXXXXX";
465465
static const int num_x = ARRAY_SIZE(x_pattern) - 1;
466-
uint64_t value;
467-
struct timeval tv;
468466
char *filename_template;
469467
size_t len;
470468
int fd, count;
@@ -485,12 +483,13 @@ int git_mkstemps_mode(char *pattern, int suffix_len, int mode)
485483
* Replace pattern's XXXXXX characters with randomness.
486484
* Try TMP_MAX different filenames.
487485
*/
488-
gettimeofday(&tv, NULL);
489-
value = ((uint64_t)tv.tv_usec << 16) ^ tv.tv_sec ^ getpid();
490486
filename_template = &pattern[len - num_x - suffix_len];
491487
for (count = 0; count < TMP_MAX; ++count) {
492-
uint64_t v = value;
493488
int i;
489+
uint64_t v;
490+
if (csprng_bytes(&v, sizeof(v)) < 0)
491+
return error_errno("unable to get random bytes for temporary file");
492+
494493
/* Fill in the random bits. */
495494
for (i = 0; i < num_x; i++) {
496495
filename_template[i] = letters[v % num_letters];
@@ -506,12 +505,6 @@ int git_mkstemps_mode(char *pattern, int suffix_len, int mode)
506505
*/
507506
if (errno != EEXIST)
508507
break;
509-
/*
510-
* This is a random value. It is only necessary that
511-
* the next TMP_MAX values generated by adding 7777 to
512-
* VALUE are different with (module 2^32).
513-
*/
514-
value += 7777;
515508
}
516509
/* We return the null string if we can't find a unique file name. */
517510
pattern[0] = '\0';

0 commit comments

Comments
 (0)