From 0bb64a7669a10e0f01082dcbe2b3afd8a1cde0b5 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Thu, 17 Jul 2025 22:33:01 +0100 Subject: [PATCH 1/2] ext/standard: Throw ValueError for filenames with null bytes This should never happen in the first place --- UPGRADING | 19 +++++++++++++++++++ ext/standard/filestat.c | 2 +- ext/standard/tests/file/bug39863.phpt | 8 ++++++-- .../tests/file/filegroup_null_byte.phpt | 14 ++++++++++++++ .../tests/file/filegroup_variation3.phpt | 12 ------------ .../tests/file/fileinode_null_byte.phpt | 14 ++++++++++++++ .../tests/file/fileinode_variation3.phpt | 12 ------------ .../tests/file/fileowner_null_byte.phpt | 14 ++++++++++++++ .../tests/file/fileowner_variation3.phpt | 12 ------------ .../tests/file/fileperms_null_byte.phpt | 14 ++++++++++++++ .../tests/file/fileperms_variation3.phpt | 12 ------------ ext/standard/tests/file/is_dir_null_byte.phpt | 14 ++++++++++++++ .../tests/file/is_dir_variation4.phpt | 10 ---------- .../tests/file/is_executable_null_byte.phpt | 14 ++++++++++++++ .../tests/file/is_executable_variation1.phpt | 13 +------------ .../tests/file/is_file_null_byte.phpt | 14 ++++++++++++++ .../tests/file/is_file_variation4.phpt | 8 -------- .../tests/file/is_readable_null_byte.phpt | 14 ++++++++++++++ .../tests/file/is_readable_variation1.phpt | 13 +------------ .../tests/file/is_writable_null_byte.phpt | 14 ++++++++++++++ .../tests/file/is_writable_variation1.phpt | 16 +--------------- 21 files changed, 155 insertions(+), 108 deletions(-) create mode 100644 ext/standard/tests/file/filegroup_null_byte.phpt create mode 100644 ext/standard/tests/file/fileinode_null_byte.phpt create mode 100644 ext/standard/tests/file/fileowner_null_byte.phpt create mode 100644 ext/standard/tests/file/fileperms_null_byte.phpt create mode 100644 ext/standard/tests/file/is_dir_null_byte.phpt create mode 100644 ext/standard/tests/file/is_executable_null_byte.phpt create mode 100644 ext/standard/tests/file/is_file_null_byte.phpt create mode 100644 ext/standard/tests/file/is_readable_null_byte.phpt create mode 100644 ext/standard/tests/file/is_writable_null_byte.phpt diff --git a/UPGRADING b/UPGRADING index 2cf8a4a4322ac..ee603a0f2d4b4 100644 --- a/UPGRADING +++ b/UPGRADING @@ -126,6 +126,25 @@ PHP 8.5 UPGRADE NOTES . Using a printf-family function with a formatter that did not specify the precision previously incorrectly reset the precision instead of treating it as a precision of 0. See GH-18897. + . Filenames with null bytes now always throw a ValueError for the following functions: + - fileperms() + - fileinode() + - filesize() + - fileowner() + - filegroup() + - fileatime() + - filemtime() + - filectime() + - filetype() + - is_writable() + - is_readable() + - is_executable() + - is_file() + - is_dir() + - is_link() + - file_exists() + - lstat() + - stat() ======================================== 2. New Features diff --git a/ext/standard/filestat.c b/ext/standard/filestat.c index 7cb54aa0aca49..df2f538f683ec 100644 --- a/ext/standard/filestat.c +++ b/ext/standard/filestat.c @@ -1018,7 +1018,7 @@ ZEND_NAMED_FUNCTION(name) { \ zend_string *filename; \ \ ZEND_PARSE_PARAMETERS_START(1, 1) \ - Z_PARAM_STR(filename) \ + Z_PARAM_PATH_STR(filename) \ ZEND_PARSE_PARAMETERS_END(); \ \ php_stat(filename, funcnum, return_value); \ diff --git a/ext/standard/tests/file/bug39863.phpt b/ext/standard/tests/file/bug39863.phpt index 88c569473a76e..2a0ca01ea65a9 100644 --- a/ext/standard/tests/file/bug39863.phpt +++ b/ext/standard/tests/file/bug39863.phpt @@ -6,7 +6,11 @@ Andrew van der Stock, vanderaj @ owasp.org getMessage(), PHP_EOL; +} ?> --EXPECT-- -bool(false) +ValueError: file_exists(): Argument #1 ($filename) must not contain any null bytes diff --git a/ext/standard/tests/file/filegroup_null_byte.phpt b/ext/standard/tests/file/filegroup_null_byte.phpt new file mode 100644 index 0000000000000..4cb80399c61dd --- /dev/null +++ b/ext/standard/tests/file/filegroup_null_byte.phpt @@ -0,0 +1,14 @@ +--TEST-- +filegroup() with filenames with null bytes +--FILE-- +getMessage(), "\n"; +} + +?> +--EXPECT-- +ValueError: filegroup(): Argument #1 ($filename) must not contain any null bytes diff --git a/ext/standard/tests/file/filegroup_variation3.phpt b/ext/standard/tests/file/filegroup_variation3.phpt index e844be0772125..94562850f58d6 100644 --- a/ext/standard/tests/file/filegroup_variation3.phpt +++ b/ext/standard/tests/file/filegroup_variation3.phpt @@ -26,10 +26,6 @@ $files_arr = array( "//filegroup_variation3//filegroup_variation3.tmp", "/filegroup_variation3/*.tmp", "filegroup_variation3/filegroup*.tmp", - - /* Testing Binary safe */ - "/filegroup_variation3/filegroup_variation3.tmp".chr(0), - "/filegroup_variation3/filegroup_variation3.tmp\0" ); $count = 1; @@ -74,13 +70,5 @@ bool(false) Warning: filegroup(): stat failed for %s/filegroup_variation3/filegroup*.tmp in %s on line %d bool(false) -- Iteration 7 - - -Warning: filegroup(): Filename contains null byte in %s on line %d -bool(false) -- Iteration 8 - - -Warning: filegroup(): Filename contains null byte in %s on line %d -bool(false) *** Done *** diff --git a/ext/standard/tests/file/fileinode_null_byte.phpt b/ext/standard/tests/file/fileinode_null_byte.phpt new file mode 100644 index 0000000000000..48b8272d97cf7 --- /dev/null +++ b/ext/standard/tests/file/fileinode_null_byte.phpt @@ -0,0 +1,14 @@ +--TEST-- +fileinode() with filenames with null bytes +--FILE-- +getMessage(), "\n"; +} + +?> +--EXPECT-- +ValueError: fileinode(): Argument #1 ($filename) must not contain any null bytes diff --git a/ext/standard/tests/file/fileinode_variation3.phpt b/ext/standard/tests/file/fileinode_variation3.phpt index 92357a3619e83..506984d417758 100644 --- a/ext/standard/tests/file/fileinode_variation3.phpt +++ b/ext/standard/tests/file/fileinode_variation3.phpt @@ -25,10 +25,6 @@ $files_arr = array( "//fileinode_variation3//fileinode_variation3.tmp", "/fileinode_variation3/*.tmp", "fileinode_variation3/fileinode*.tmp", - - /* Testing Binary safe */ - "/fileinode_variation3/fileinode_variation3.tmp".chr(0), - "/fileinode_variation3/fileinode_variation3.tmp\0" ); $count = 1; @@ -73,13 +69,5 @@ bool(false) Warning: fileinode(): stat failed for %s/fileinode_variation3/fileinode*.tmp in %s on line %d bool(false) -- Iteration 7 - - -Warning: fileinode(): Filename contains null byte in %s on line %d -bool(false) -- Iteration 8 - - -Warning: fileinode(): Filename contains null byte in %s on line %d -bool(false) *** Done *** diff --git a/ext/standard/tests/file/fileowner_null_byte.phpt b/ext/standard/tests/file/fileowner_null_byte.phpt new file mode 100644 index 0000000000000..909ac0fe31091 --- /dev/null +++ b/ext/standard/tests/file/fileowner_null_byte.phpt @@ -0,0 +1,14 @@ +--TEST-- +fileowner() with filenames with null bytes +--FILE-- +getMessage(), "\n"; +} + +?> +--EXPECT-- +ValueError: fileowner(): Argument #1 ($filename) must not contain any null bytes diff --git a/ext/standard/tests/file/fileowner_variation3.phpt b/ext/standard/tests/file/fileowner_variation3.phpt index 63ba6936aaa8d..e6360b4757137 100644 --- a/ext/standard/tests/file/fileowner_variation3.phpt +++ b/ext/standard/tests/file/fileowner_variation3.phpt @@ -26,10 +26,6 @@ $files_arr = array( "//fileowner_variation3//fileowner_variation3.tmp", "/fileowner_variation3/*.tmp", "fileowner_variation3/fileowner*.tmp", - - /* Testing Binary safe */ - "/fileowner_variation3/fileowner_variation3.tmp".chr(0), - "/fileowner_variation3/fileowner_variation3.tmp\0" ); $count = 1; @@ -74,13 +70,5 @@ bool(false) Warning: fileowner(): stat failed for %s/fileowner_variation3/fileowner*.tmp in %s on line %d bool(false) -- Iteration 7 - - -Warning: fileowner(): Filename contains null byte in %s on line %d -bool(false) -- Iteration 8 - - -Warning: fileowner(): Filename contains null byte in %s on line %d -bool(false) *** Done *** diff --git a/ext/standard/tests/file/fileperms_null_byte.phpt b/ext/standard/tests/file/fileperms_null_byte.phpt new file mode 100644 index 0000000000000..1530d0e5bf9f2 --- /dev/null +++ b/ext/standard/tests/file/fileperms_null_byte.phpt @@ -0,0 +1,14 @@ +--TEST-- +fileperms() with filenames with null bytes +--FILE-- +getMessage(), "\n"; +} + +?> +--EXPECT-- +ValueError: fileperms(): Argument #1 ($filename) must not contain any null bytes diff --git a/ext/standard/tests/file/fileperms_variation3.phpt b/ext/standard/tests/file/fileperms_variation3.phpt index 5e981e9b86320..6c1da17d6bfd9 100644 --- a/ext/standard/tests/file/fileperms_variation3.phpt +++ b/ext/standard/tests/file/fileperms_variation3.phpt @@ -25,10 +25,6 @@ $files_arr = array( "//fileperms_variation3//fileperms_variation3.tmp", "/fileperms_variation3/*.tmp", "fileperms_variation3/fileperms*.tmp", - - /* Testing Binary safe */ - "/fileperms_variation3/fileperms_variation3.tmp".chr(0), - "/fileperms_variation3/fileperms_variation3.tmp\0" ); $count = 1; @@ -73,13 +69,5 @@ bool(false) Warning: fileperms(): stat failed for %s/fileperms_variation3/fileperms*.tmp in %s on line %d bool(false) -- Iteration 7 - - -Warning: fileperms(): Filename contains null byte in %s on line %d -bool(false) -- Iteration 8 - - -Warning: fileperms(): Filename contains null byte in %s on line %d -bool(false) *** Done *** diff --git a/ext/standard/tests/file/is_dir_null_byte.phpt b/ext/standard/tests/file/is_dir_null_byte.phpt new file mode 100644 index 0000000000000..a49ccc0e5c45a --- /dev/null +++ b/ext/standard/tests/file/is_dir_null_byte.phpt @@ -0,0 +1,14 @@ +--TEST-- +is_dir() with filenames with null bytes +--FILE-- +getMessage(), "\n"; +} + +?> +--EXPECT-- +ValueError: is_dir(): Argument #1 ($filename) must not contain any null bytes diff --git a/ext/standard/tests/file/is_dir_variation4.phpt b/ext/standard/tests/file/is_dir_variation4.phpt index d6e64efe211cd..30285804a33cc 100644 --- a/ext/standard/tests/file/is_dir_variation4.phpt +++ b/ext/standard/tests/file/is_dir_variation4.phpt @@ -23,10 +23,6 @@ $dirs_arr = array( "./is_dir_variation4//", ".//is_dir_variation4//", "is_dir_vari*", - - /* Testing Binary safe */ - "./is_dir_variation4/".chr(0), - "is_dir_variation4\0" ); $count = 1; @@ -75,10 +71,4 @@ bool(true) -- Iteration 8 -- bool(false) --- Iteration 9 -- -bool(false) - --- Iteration 10 -- -bool(false) - *** Done *** diff --git a/ext/standard/tests/file/is_executable_null_byte.phpt b/ext/standard/tests/file/is_executable_null_byte.phpt new file mode 100644 index 0000000000000..7c26d53061d1f --- /dev/null +++ b/ext/standard/tests/file/is_executable_null_byte.phpt @@ -0,0 +1,14 @@ +--TEST-- +is_executable() with filenames with null bytes +--FILE-- +getMessage(), "\n"; +} + +?> +--EXPECT-- +ValueError: is_executable(): Argument #1 ($filename) must not contain any null bytes diff --git a/ext/standard/tests/file/is_executable_variation1.phpt b/ext/standard/tests/file/is_executable_variation1.phpt index a85b9a2ef0283..289abb1a265f3 100644 --- a/ext/standard/tests/file/is_executable_variation1.phpt +++ b/ext/standard/tests/file/is_executable_variation1.phpt @@ -33,11 +33,6 @@ $files_arr = array( "$file_path/is_executable_variation1/*.tmp", "$file_path/is_executable_variation1/b*.tmp", - /* Testing Binary safe */ - "$file_path/is_executable_variation1".chr(0)."bar.temp", - "$file_path".chr(0)."is_executable_variation1/bar.temp", - "$file_path/is_executable_variation1x000/", - /* Testing directories */ ".", // current directory, exp: bool(true) "$file_path/is_executable_variation1" // temp directory, exp: bool(true) @@ -76,13 +71,7 @@ bool(false) -- Iteration 5 -- bool(false) -- Iteration 6 -- -bool(false) --- Iteration 7 -- -bool(false) --- Iteration 8 -- -bool(false) --- Iteration 9 -- bool(true) --- Iteration 10 -- +-- Iteration 7 -- bool(true) Done diff --git a/ext/standard/tests/file/is_file_null_byte.phpt b/ext/standard/tests/file/is_file_null_byte.phpt new file mode 100644 index 0000000000000..f544d7bcf5894 --- /dev/null +++ b/ext/standard/tests/file/is_file_null_byte.phpt @@ -0,0 +1,14 @@ +--TEST-- +is_file() with filenames with null bytes +--FILE-- +getMessage(), "\n"; +} + +?> +--EXPECT-- +ValueError: is_file(): Argument #1 ($filename) must not contain any null bytes diff --git a/ext/standard/tests/file/is_file_variation4.phpt b/ext/standard/tests/file/is_file_variation4.phpt index 1afc34dd03bea..9d98b7fd90d73 100644 --- a/ext/standard/tests/file/is_file_variation4.phpt +++ b/ext/standard/tests/file/is_file_variation4.phpt @@ -23,10 +23,6 @@ $files_arr = array( "//is_file_variation4//is_file_variation4.tmp", "/is_file_variation4/*.tmp", "is_file_variation4/is_file*.tmp", - - /* Testing Binary safe */ - "/is_file_variation4/is_file_variation4.tmp".chr(0), - "/is_file_variation4/is_file_variation4.tmp\0" ); $count = 1; @@ -65,9 +61,5 @@ bool(true) bool(false) - Iteration 6 - bool(false) -- Iteration 7 - -bool(false) -- Iteration 8 - -bool(false) *** Done *** diff --git a/ext/standard/tests/file/is_readable_null_byte.phpt b/ext/standard/tests/file/is_readable_null_byte.phpt new file mode 100644 index 0000000000000..711700f4e7ec2 --- /dev/null +++ b/ext/standard/tests/file/is_readable_null_byte.phpt @@ -0,0 +1,14 @@ +--TEST-- +is_readable() with filenames with null bytes +--FILE-- +getMessage(), "\n"; +} + +?> +--EXPECT-- +ValueError: is_readable(): Argument #1 ($filename) must not contain any null bytes diff --git a/ext/standard/tests/file/is_readable_variation1.phpt b/ext/standard/tests/file/is_readable_variation1.phpt index 9c25213f8abb2..8548a840ccbe7 100644 --- a/ext/standard/tests/file/is_readable_variation1.phpt +++ b/ext/standard/tests/file/is_readable_variation1.phpt @@ -32,11 +32,6 @@ $files_arr = array( "$file_path/is_readable_variation1/*.tmp", "$file_path/is_readable_variation1/b*.tmp", - /* Testing Binary safe */ - "$file_path/is_readable_variation1".chr(0)."bar.tmp", - "$file_path".chr(0)."is_readable_variation1/bar.tmp", - "$file_path".chr(0)."is_readable_variation1/bar.tmp", - /* Testing directories */ ".", // current directory, exp: bool(true) "$file_path/is_readable_variation1" // temp directory, exp: bool(true) @@ -77,13 +72,7 @@ bool(false) -- Iteration 6 -- bool(false) -- Iteration 7 -- -bool(false) --- Iteration 8 -- -bool(false) --- Iteration 9 -- -bool(false) --- Iteration 10 -- bool(true) --- Iteration 11 -- +-- Iteration 8 -- bool(true) Done diff --git a/ext/standard/tests/file/is_writable_null_byte.phpt b/ext/standard/tests/file/is_writable_null_byte.phpt new file mode 100644 index 0000000000000..319db04cc151a --- /dev/null +++ b/ext/standard/tests/file/is_writable_null_byte.phpt @@ -0,0 +1,14 @@ +--TEST-- +is_writable() with filenames with null bytes +--FILE-- +getMessage(), "\n"; +} + +?> +--EXPECT-- +ValueError: is_writable(): Argument #1 ($filename) must not contain any null bytes diff --git a/ext/standard/tests/file/is_writable_variation1.phpt b/ext/standard/tests/file/is_writable_variation1.phpt index 80695d6d45a25..181b1ba4e2689 100644 --- a/ext/standard/tests/file/is_writable_variation1.phpt +++ b/ext/standard/tests/file/is_writable_variation1.phpt @@ -31,11 +31,6 @@ $files_arr = array( "$file_path/is_writable_variation1/*.tmp", "$file_path/is_writable_variation1/b*.tmp", - /* Testing Binary safe */ - "$file_path/is_writable_variation1".chr(0)."bar.tmp", - "$file_path".chr(0)."is_writable_variation1/bar.tmp", - "$file_path".chr(0)."is_writable_variation1/bar.tmp", - /* Testing directories */ ".", // current directory, exp: bool(true) "$file_path/is_writable_variation1" // temp directory, exp: bool(true) @@ -87,18 +82,9 @@ bool(false) bool(false) bool(false) -- Iteration 7 -- -bool(false) -bool(false) --- Iteration 8 -- -bool(false) -bool(false) --- Iteration 9 -- -bool(false) -bool(false) --- Iteration 10 -- bool(true) bool(true) --- Iteration 11 -- +-- Iteration 8 -- bool(true) bool(true) Done From 782278abf66eadb1e60b953690711c4758c94661 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Thu, 17 Jul 2025 22:40:05 +0100 Subject: [PATCH 2/2] ext/standard: Refactor php_stat() API --- UPGRADING.INTERNALS | 4 ++++ ext/standard/filestat.c | 19 +++++-------------- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index 0e0a0e94c9477..51b0663201fa8 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -134,6 +134,10 @@ PHP 8.5 INTERNALS UPGRADE NOTES . The php_std_date() function has been removed. Use php_format_date() with the "D, d M Y H:i:s \\G\\M\\T" format instead. . Added php_url_encode_to_smart_str() to encode a URL to a smart_str buffer. + . The php_stat() function now asserts that the zend_string *filename + parameter does not contain any null bytes. To ensure such strings are not + provided the zend_str_has_nul_byte() API, the P ZPP specifier, or the fast + ZPP Z_PARAM_PATH_STR specifier can be used to check this ahead of time. ======================== 4. OpCode changes diff --git a/ext/standard/filestat.c b/ext/standard/filestat.c index df2f538f683ec..5193db37437d6 100644 --- a/ext/standard/filestat.c +++ b/ext/standard/filestat.c @@ -748,14 +748,12 @@ PHPAPI void php_stat(zend_string *filename, int type, zval *return_value) const char *local = NULL; php_stream_wrapper *wrapper = NULL; + ZEND_ASSERT(!zend_str_has_nul_byte(filename)); + /* Quick check for empty file paths */ + if (!ZSTR_LEN(filename)) { + RETURN_FALSE; + } if (IS_ACCESS_CHECK(type)) { - if (!ZSTR_LEN(filename) || CHECK_NULL_PATH(ZSTR_VAL(filename), ZSTR_LEN(filename))) { - if (ZSTR_LEN(filename) && !IS_EXISTS_CHECK(type)) { - php_error_docref(NULL, E_WARNING, "Filename contains null byte"); - } - RETURN_FALSE; - } - if ((wrapper = php_stream_locate_url_wrapper(ZSTR_VAL(filename), &local, 0)) == &php_plain_files_wrapper && php_check_open_basedir(local)) { RETURN_FALSE; @@ -821,13 +819,6 @@ PHPAPI void php_stat(zend_string *filename, int type, zval *return_value) } if (!wrapper) { - if (!ZSTR_LEN(filename) || CHECK_NULL_PATH(ZSTR_VAL(filename), ZSTR_LEN(filename))) { - if (ZSTR_LEN(filename) && !IS_EXISTS_CHECK(type)) { - php_error_docref(NULL, E_WARNING, "Filename contains null byte"); - } - RETURN_FALSE; - } - if ((wrapper = php_stream_locate_url_wrapper(ZSTR_VAL(filename), &local, 0)) == &php_plain_files_wrapper && php_check_open_basedir(local)) { RETURN_FALSE;