Skip to content

Improve randomness of uploaded file names and files created by tempnam() #14364

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

Improve randomness of uploaded file names and files created by tempnam() #14364

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d169bc2
Improve randomness of uploaded file names and files created by tempnam()
arnaud-lb May 29, 2024
3303a2c
Revert to 64bits of randomness for compatibility
arnaud-lb May 30, 2024
53fa483
Case-insensitive encoding
arnaud-lb May 30, 2024
44d33c0
Use base32 so mod and div get optimized to mask and shift
arnaud-lb May 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions ext/standard/tests/file/tempnam_variation9.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,17 @@ if (file_exists($file_path)) {
--EXPECTF--
*** Testing tempnam() maximum prefix size ***
-- Iteration 0 --
File name is => begin_%rx{7}%r_end%r.{6}%r
File name length is => 23
File name is => begin_%rx{7}%r_end%r.{19}%r
File name length is => 36
-- Iteration 1 --
File name is => begin_%rx{53}%r_end%r.{6}%r
File name length is => 69
File name is => begin_%rx{53}%r_end%r.{19}%r
File name length is => 82
-- Iteration 2 --
File name is => begin_%rx{54}%r_en%r.{6}%r
File name length is => 69
File name is => begin_%rx{54}%r_en%r.{19}%r
File name length is => 82
-- Iteration 3 --
File name is => begin_%rx{55}%r_e%r.{6}%r
File name length is => 69
File name is => begin_%rx{55}%r_e%r.{19}%r
File name length is => 82
-- Iteration 4 --
File name is => begin_%rx{57}%r%r.{6}%r
File name length is => 69
File name is => begin_%rx{57}%r%r.{19}%r
File name length is => 82
49 changes: 40 additions & 9 deletions main/php_open_temporary_file.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@
*/

#include "php.h"
#include "zend_long.h"
#include "php_open_temporary_file.h"
#include "ext/random/php_random.h"
#include "zend_operators.h"

#include <errno.h>
#include <sys/types.h>
Expand Down Expand Up @@ -84,16 +87,22 @@
* SUCH DAMAGE.
*/

static const char base32alphabet[] = "0123456789abcdefghijklmnopqrstuv";

static int php_do_open_temporary_file(const char *path, const char *pfx, zend_string **opened_path_p)
{
#ifdef PHP_WIN32
char *opened_path = NULL;
size_t opened_path_len;
wchar_t *cwdw, *pfxw, pathw[MAXPATHLEN];
wchar_t *cwdw, *random_prefix_w, pathw[MAXPATHLEN];
#else
char opened_path[MAXPATHLEN];
char *trailing_slash;
#endif
uint64_t random;
char *random_prefix;
char *p;
size_t len;
char cwd[MAXPATHLEN];
cwd_state new_state;
int fd = -1;
Expand Down Expand Up @@ -128,34 +137,54 @@ static int php_do_open_temporary_file(const char *path, const char *pfx, zend_st
return -1;
}

/* Extend the prefix to increase randomness */
if (php_random_bytes_silent(&random, sizeof(random)) == FAILURE) {
random = php_random_generate_fallback_seed();
}

/* Use a compact encoding to not increase the path len too much, but do not
* mix case to avoid losing randomness on case-insensitive file systems */
len = strlen(pfx) + 13 /* log(2**64)/log(strlen(letters)) */ + 1;
random_prefix = emalloc(len);
p = zend_mempcpy(random_prefix, pfx, strlen(pfx));
while (p + 1 < random_prefix + len) {
*p = base32alphabet[random % strlen(base32alphabet)];
p++;
random /= strlen(base32alphabet);
}
*p = '\0';

#ifndef PHP_WIN32
if (IS_SLASH(new_state.cwd[new_state.cwd_length - 1])) {
trailing_slash = "";
} else {
trailing_slash = "/";
}

if (snprintf(opened_path, MAXPATHLEN, "%s%s%sXXXXXX", new_state.cwd, trailing_slash, pfx) >= MAXPATHLEN) {
if (snprintf(opened_path, MAXPATHLEN, "%s%s%sXXXXXX", new_state.cwd, trailing_slash, random_prefix) >= MAXPATHLEN) {
efree(random_prefix);
efree(new_state.cwd);
return -1;
}
#endif

#ifdef PHP_WIN32
cwdw = php_win32_ioutil_any_to_w(new_state.cwd);
pfxw = php_win32_ioutil_any_to_w(pfx);
if (!cwdw || !pfxw) {
random_prefix_w = php_win32_ioutil_any_to_w(random_prefix);
if (!cwdw || !random_prefix_w) {
free(cwdw);
free(pfxw);
free(random_prefix_w);
efree(random_prefix);
efree(new_state.cwd);
return -1;
}

if (GetTempFileNameW(cwdw, pfxw, 0, pathw)) {
if (GetTempFileNameW(cwdw, random_prefix_w, 0, pathw)) {
opened_path = php_win32_ioutil_conv_w_to_any(pathw, PHP_WIN32_CP_IGNORE_LEN, &opened_path_len);
if (!opened_path || opened_path_len >= MAXPATHLEN) {
free(cwdw);
free(pfxw);
free(random_prefix_w);
efree(random_prefix);
efree(new_state.cwd);
return -1;
}
Expand All @@ -165,7 +194,8 @@ static int php_do_open_temporary_file(const char *path, const char *pfx, zend_st
* which means that opening it will fail... */
if (VCWD_CHMOD(opened_path, 0600)) {
free(cwdw);
free(pfxw);
free(random_prefix_w);
efree(random_prefix);
efree(new_state.cwd);
free(opened_path);
return -1;
Expand All @@ -174,7 +204,7 @@ static int php_do_open_temporary_file(const char *path, const char *pfx, zend_st
}

free(cwdw);
free(pfxw);
free(random_prefix_w);
#elif defined(HAVE_MKSTEMP)
fd = mkstemp(opened_path);
#else
Expand All @@ -194,6 +224,7 @@ static int php_do_open_temporary_file(const char *path, const char *pfx, zend_st
}
#endif
efree(new_state.cwd);
efree(random_prefix);
return fd;
}
/* }}} */
Expand Down
Loading