From 4d41dffb4f355872476bae561264c4796eef26b8 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 18 Nov 2023 23:50:53 +0100 Subject: [PATCH] Fix GH-12721: SplFileInfo::getFilename() segfault in combination with GlobIterator and no directory separator This broke in 7cd8879 and 9bae9ab. NULL is a perfectly valid return value that should be handled. Closes GH-12722. --- NEWS | 4 +++ ext/spl/spl_directory.c | 15 +++++++---- ext/spl/tests/gh12721.phpt | 51 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 5 deletions(-) create mode 100644 ext/spl/tests/gh12721.phpt diff --git a/NEWS b/NEWS index 45805551eed..3bf074b1e1b 100644 --- a/NEWS +++ b/NEWS @@ -22,6 +22,10 @@ PHP NEWS - PHPDBG: . Fixed bug GH-12675 (MEMORY_LEAK in phpdbg_prompt.c). (nielsdos) +- SPL: + . Fixed bug GH-12721 (SplFileInfo::getFilename() segfault in combination + with GlobIterator and no directory separator). (nielsdos) + - SQLite3: . Fixed bug GH-12633 (sqlite3_defensive.phpt fails with sqlite 3.44.0). (SakiTakamachi) diff --git a/ext/spl/spl_directory.c b/ext/spl/spl_directory.c index 0dcb34b6a69..581183d38f4 100644 --- a/ext/spl/spl_directory.c +++ b/ext/spl/spl_directory.c @@ -925,7 +925,6 @@ PHP_METHOD(SplFileInfo, getFilename) path = spl_filesystem_object_get_path(intern); - ZEND_ASSERT(path); if (path && ZSTR_LEN(path) && ZSTR_LEN(path) < ZSTR_LEN(intern->file_name)) { /* +1 to skip the trailing / of the path in the file name */ size_t path_len = ZSTR_LEN(path) + 1; @@ -933,7 +932,9 @@ PHP_METHOD(SplFileInfo, getFilename) } else { RETVAL_STR_COPY(intern->file_name); } - zend_string_release_ex(path, /* persistent */ false); + if (path) { + zend_string_release_ex(path, /* persistent */ false); + } } /* }}} */ @@ -973,14 +974,16 @@ PHP_METHOD(SplFileInfo, getExtension) path = spl_filesystem_object_get_path(intern); - if (ZSTR_LEN(path) && ZSTR_LEN(path) < ZSTR_LEN(intern->file_name)) { + if (path && ZSTR_LEN(path) && ZSTR_LEN(path) < ZSTR_LEN(intern->file_name)) { fname = ZSTR_VAL(intern->file_name) + ZSTR_LEN(path) + 1; flen = ZSTR_LEN(intern->file_name) - (ZSTR_LEN(path) + 1); } else { fname = ZSTR_VAL(intern->file_name); flen = ZSTR_LEN(intern->file_name); } - zend_string_release_ex(path, /* persistent */ false); + if (path) { + zend_string_release_ex(path, /* persistent */ false); + } ret = php_basename(fname, flen, NULL, 0); @@ -1052,7 +1055,9 @@ PHP_METHOD(SplFileInfo, getBasename) fname = ZSTR_VAL(intern->file_name); flen = ZSTR_LEN(intern->file_name); } - zend_string_release_ex(path, /* persistent */ false); + if (path) { + zend_string_release_ex(path, /* persistent */ false); + } RETURN_STR(php_basename(fname, flen, suffix, slen)); } diff --git a/ext/spl/tests/gh12721.phpt b/ext/spl/tests/gh12721.phpt new file mode 100644 index 00000000000..4310afcdd93 --- /dev/null +++ b/ext/spl/tests/gh12721.phpt @@ -0,0 +1,51 @@ +--TEST-- +GH-12721 (SplFileInfo::getFilename() segfault in combination with GlobIterator and no directory separator) +--FILE-- +getFilename(), "\n"; + echo $fileInfo->getExtension(), "\n"; + echo $fileInfo->getBasename(), "\n"; + var_dump($fileInfo->getFileInfo()); +} + +echo "--- With slash ---\n"; + +foreach (new GlobIterator('./*.gh12721') as $fileInfo) { + echo $fileInfo->getFilename(), "\n"; + echo $fileInfo->getExtension(), "\n"; + echo $fileInfo->getBasename(), "\n"; + var_dump($fileInfo->getFileInfo()); +} + +?> +--CLEAN-- + +--EXPECTF-- +--- No slash --- +file1.gh12721 +gh12721 +file1.gh12721 +object(SplFileInfo)#4 (2) { + ["pathName":"SplFileInfo":private]=> + string(13) "file1.gh12721" + ["fileName":"SplFileInfo":private]=> + string(13) "file1.gh12721" +} +--- With slash --- +file1.gh12721 +gh12721 +file1.gh12721 +object(SplFileInfo)#3 (2) { + ["pathName":"SplFileInfo":private]=> + string(15) "%sfile1.gh12721" + ["fileName":"SplFileInfo":private]=> + string(13) "file1.gh12721" +}