Skip to content

ext/standard: Check for null bytes in filenames and refactor php_stat() #19165

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.

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 19 additions & 0 deletions UPGRADING
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions UPGRADING.INTERNALS
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 6 additions & 15 deletions ext/standard/filestat.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -1018,7 +1009,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); \
Expand Down
8 changes: 6 additions & 2 deletions ext/standard/tests/file/bug39863.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ Andrew van der Stock, vanderaj @ owasp.org
<?php

$filename = __FILE__ . chr(0). ".ridiculous";
var_dump(file_exists($filename));
try {
var_dump(file_exists($filename));
} catch (Throwable $e) {
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
}
?>
--EXPECT--
bool(false)
ValueError: file_exists(): Argument #1 ($filename) must not contain any null bytes
14 changes: 14 additions & 0 deletions ext/standard/tests/file/filegroup_null_byte.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
filegroup() with filenames with null bytes
--FILE--
<?php

try {
var_dump(filegroup("file_with_null_byte.tmp\0"));
} catch (Throwable $e) {
echo $e::class, ': ', $e->getMessage(), "\n";
}

?>
--EXPECT--
ValueError: filegroup(): Argument #1 ($filename) must not contain any null bytes
12 changes: 0 additions & 12 deletions ext/standard/tests/file/filegroup_variation3.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 ***
14 changes: 14 additions & 0 deletions ext/standard/tests/file/fileinode_null_byte.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
fileinode() with filenames with null bytes
--FILE--
<?php

try {
var_dump(fileinode("file_with_null_byte.tmp\0"));
} catch (Throwable $e) {
echo $e::class, ': ', $e->getMessage(), "\n";
}

?>
--EXPECT--
ValueError: fileinode(): Argument #1 ($filename) must not contain any null bytes
12 changes: 0 additions & 12 deletions ext/standard/tests/file/fileinode_variation3.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 ***
14 changes: 14 additions & 0 deletions ext/standard/tests/file/fileowner_null_byte.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
fileowner() with filenames with null bytes
--FILE--
<?php

try {
var_dump(fileowner("file_with_null_byte.tmp\0"));
} catch (Throwable $e) {
echo $e::class, ': ', $e->getMessage(), "\n";
}

?>
--EXPECT--
ValueError: fileowner(): Argument #1 ($filename) must not contain any null bytes
12 changes: 0 additions & 12 deletions ext/standard/tests/file/fileowner_variation3.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 ***
14 changes: 14 additions & 0 deletions ext/standard/tests/file/fileperms_null_byte.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
fileperms() with filenames with null bytes
--FILE--
<?php

try {
var_dump(fileperms("file_with_null_byte.tmp\0"));
} catch (Throwable $e) {
echo $e::class, ': ', $e->getMessage(), "\n";
}

?>
--EXPECT--
ValueError: fileperms(): Argument #1 ($filename) must not contain any null bytes
12 changes: 0 additions & 12 deletions ext/standard/tests/file/fileperms_variation3.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 ***
14 changes: 14 additions & 0 deletions ext/standard/tests/file/is_dir_null_byte.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
is_dir() with filenames with null bytes
--FILE--
<?php

try {
var_dump(is_dir("file_with_null_byte.tmp\0"));
} catch (Throwable $e) {
echo $e::class, ': ', $e->getMessage(), "\n";
}

?>
--EXPECT--
ValueError: is_dir(): Argument #1 ($filename) must not contain any null bytes
10 changes: 0 additions & 10 deletions ext/standard/tests/file/is_dir_variation4.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -75,10 +71,4 @@ bool(true)
-- Iteration 8 --
bool(false)

-- Iteration 9 --
bool(false)

-- Iteration 10 --
bool(false)

*** Done ***
14 changes: 14 additions & 0 deletions ext/standard/tests/file/is_executable_null_byte.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
is_executable() with filenames with null bytes
--FILE--
<?php

try {
var_dump(is_executable("file_with_null_byte.tmp\0"));
} catch (Throwable $e) {
echo $e::class, ': ', $e->getMessage(), "\n";
}

?>
--EXPECT--
ValueError: is_executable(): Argument #1 ($filename) must not contain any null bytes
13 changes: 1 addition & 12 deletions ext/standard/tests/file/is_executable_variation1.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
14 changes: 14 additions & 0 deletions ext/standard/tests/file/is_file_null_byte.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
is_file() with filenames with null bytes
--FILE--
<?php

try {
var_dump(is_file("file_with_null_byte.tmp\0"));
} catch (Throwable $e) {
echo $e::class, ': ', $e->getMessage(), "\n";
}

?>
--EXPECT--
ValueError: is_file(): Argument #1 ($filename) must not contain any null bytes
8 changes: 0 additions & 8 deletions ext/standard/tests/file/is_file_variation4.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -65,9 +61,5 @@ bool(true)
bool(false)
- Iteration 6 -
bool(false)
- Iteration 7 -
bool(false)
- Iteration 8 -
bool(false)

*** Done ***
14 changes: 14 additions & 0 deletions ext/standard/tests/file/is_readable_null_byte.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
is_readable() with filenames with null bytes
--FILE--
<?php

try {
var_dump(is_readable("file_with_null_byte.tmp\0"));
} catch (Throwable $e) {
echo $e::class, ': ', $e->getMessage(), "\n";
}

?>
--EXPECT--
ValueError: is_readable(): Argument #1 ($filename) must not contain any null bytes
Loading
Loading