1
0
mirror of https://github.com/php/php-src.git synced 2026-03-24 00:02:20 +01:00

Fix GH-17225: NULL deref in spl_directory.c

NULL checks for the glob stream are inconsistently applied. To solve
this generally, factor it out to a helper function so it's less likely
to be forgotten in the future.

Closes GH-17231.
This commit is contained in:
Niels Dossche
2024-12-21 12:11:21 +01:00
parent 61615d5673
commit 4bfe69bbc4
3 changed files with 66 additions and 5 deletions

3
NEWS
View File

@@ -72,6 +72,9 @@ PHP NEWS
on SO_SNDTIMEO/SO_RCVTIMEO for socket_set_option().
(David Carlier)
- SPL:
. Fixed bug GH-17225 (NULL deref in spl_directory.c). (nielsdos)
- Streams:
. Fixed bug GH-17037 (UAF in user filter when adding existing filter name due
to incorrect error handling). (nielsdos)

View File

@@ -212,10 +212,16 @@ static zend_object *spl_filesystem_object_new(zend_class_entry *class_type)
}
/* }}} */
static inline bool spl_intern_is_glob(const spl_filesystem_object *intern)
{
/* NULL check on `dirp` is necessary as destructors may interfere. */
return intern->u.dir.dirp && php_stream_is(intern->u.dir.dirp, &php_glob_stream_ops);
}
PHPAPI zend_string *spl_filesystem_object_get_path(spl_filesystem_object *intern) /* {{{ */
{
#ifdef HAVE_GLOB
if (intern->type == SPL_FS_DIR && php_stream_is(intern->u.dir.dirp, &php_glob_stream_ops)) {
if (intern->type == SPL_FS_DIR && spl_intern_is_glob(intern)) {
size_t len = 0;
char *tmp = php_glob_stream_get_path(intern->u.dir.dirp, &len);
if (len == 0) {
@@ -658,7 +664,7 @@ static inline HashTable *spl_filesystem_object_get_debug_info(zend_object *objec
if (intern->type == SPL_FS_DIR) {
#ifdef HAVE_GLOB
pnstr = spl_gen_private_prop_name(spl_ce_DirectoryIterator, "glob", sizeof("glob")-1);
if (intern->u.dir.dirp && php_stream_is(intern->u.dir.dirp ,&php_glob_stream_ops)) {
if (spl_intern_is_glob(intern)) {
ZVAL_STR_COPY(&tmp, intern->path);
} else {
ZVAL_FALSE(&tmp);
@@ -1614,11 +1620,11 @@ PHP_METHOD(GlobIterator, count)
RETURN_THROWS();
}
if (intern->u.dir.dirp && php_stream_is(intern->u.dir.dirp ,&php_glob_stream_ops)) {
if (spl_intern_is_glob(intern)) {
RETURN_LONG(php_glob_stream_get_count(intern->u.dir.dirp, NULL));
} else {
/* should not happen */
// TODO ZEND_ASSERT ?
/* This can happen by abusing destructors. */
/* TODO: relax this from E_ERROR to an exception */
php_error_docref(NULL, E_ERROR, "GlobIterator lost glob state");
}
}

View File

@@ -0,0 +1,52 @@
--TEST--
GH-17225 (NULL deref in spl_directory.c)
--CREDITS--
YuanchengJiang
--EXTENSIONS--
phar
--INI--
phar.readonly=0
--FILE--
<?php
$fname = __DIR__ . '/gh17225.phar.zip';
$phar = new Phar($fname);
class HasDestructor {
public function __destruct() {
var_dump($GLOBALS['s']);
}
}
$s = new SplObjectStorage();
$s[$phar] = new HasDestructor();
register_shutdown_function(function() {
global $s;
});
var_dump($phar->isLink());
?>
--CLEAN--
<?php
@unlink(__DIR__ . '/gh17225.phar.zip');
?>
--EXPECTF--
bool(false)
object(SplObjectStorage)#%d (1) {
["storage":"SplObjectStorage":private]=>
array(1) {
[0]=>
array(2) {
["obj"]=>
object(Phar)#%d (4) {
["pathName":"SplFileInfo":private]=>
string(0) ""
["fileName":"SplFileInfo":private]=>
string(0) ""
["glob":"DirectoryIterator":private]=>
bool(false)
["subPathName":"RecursiveDirectoryIterator":private]=>
string(0) ""
}
["inf"]=>
object(HasDestructor)#%d (0) {
}
}
}
}