From b4325d6113d38ba5e9d5711006f2da349f20a469 Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Wed, 29 May 2024 19:06:18 +0200 Subject: [PATCH] Improve randomness of uploaded file names and files created by tempnam() Closes GH-14364 --- NEWS | 2 + UPGRADING | 4 +- .../tests/file/tempnam_variation9.phpt | 20 ++++---- main/php_open_temporary_file.c | 49 +++++++++++++++---- 4 files changed, 55 insertions(+), 20 deletions(-) diff --git a/NEWS b/NEWS index e2bcc21e002..9a740d450c9 100644 --- a/NEWS +++ b/NEWS @@ -24,6 +24,8 @@ PHP NEWS . Fixed GH-13581 no space available for TLS on NetBSD. (Paul Ripke) . Added fiber Sys-V loongarch64 support. (qiangxuhui) . Adjusted closure names to include the parent function's name. (timwolla) + . Improve randomness of uploaded file names and files created by tempnam(). + (Arnaud) - Curl: . Deprecated the CURLOPT_BINARYTRANSFER constant. (divinity76) diff --git a/UPGRADING b/UPGRADING index 73dea4f4d33..a03171c4411 100644 --- a/UPGRADING +++ b/UPGRADING @@ -29,6 +29,8 @@ PHP 8.4 UPGRADE NOTES - Core: . The type of PHP_DEBUG and PHP_ZTS constants changed to bool. + . The name of uploaded files and files created by the tempnam() function are + now 13 bytes longer. Total length is platform-dependent. - DOM: . Added DOMNode::compareDocumentPosition() and DOMNode::DOCUMENT_POSITION_* @@ -237,7 +239,7 @@ PHP 8.4 UPGRADE NOTES ed25519, x448 and ed448 fields are supported in openssl_pkey_new and openssl_pkey_get_details as well as openssl_sign and openssl_verify were extended to support those keys. - + - Phar: . Added support for the unix timestamp extension for zip archives. diff --git a/ext/standard/tests/file/tempnam_variation9.phpt b/ext/standard/tests/file/tempnam_variation9.phpt index 22f3ea109bf..61178a022ce 100644 --- a/ext/standard/tests/file/tempnam_variation9.phpt +++ b/ext/standard/tests/file/tempnam_variation9.phpt @@ -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 diff --git a/main/php_open_temporary_file.c b/main/php_open_temporary_file.c index 07aaaaa3d9a..b45537935a5 100644 --- a/main/php_open_temporary_file.c +++ b/main/php_open_temporary_file.c @@ -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 #include @@ -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; @@ -128,6 +137,23 @@ 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(base32alphabet)) */ + 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 = ""; @@ -135,7 +161,8 @@ static int php_do_open_temporary_file(const char *path, const char *pfx, zend_st 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; } @@ -143,19 +170,21 @@ static int php_do_open_temporary_file(const char *path, const char *pfx, zend_st #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; } @@ -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; @@ -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 @@ -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; } /* }}} */