diff --git a/ext/spl/spl_directory.c b/ext/spl/spl_directory.c index 650439dbc8939..29f4108794cd9 100644 --- a/ext/spl/spl_directory.c +++ b/ext/spl/spl_directory.c @@ -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"); } } diff --git a/ext/spl/tests/gh17225.phpt b/ext/spl/tests/gh17225.phpt new file mode 100644 index 0000000000000..66a52bce7f4a7 --- /dev/null +++ b/ext/spl/tests/gh17225.phpt @@ -0,0 +1,52 @@ +--TEST-- +GH-17225 (NULL deref in spl_directory.c) +--CREDITS-- +YuanchengJiang +--EXTENSIONS-- +phar +--INI-- +phar.readonly=0 +--FILE-- +isLink()); +?> +--CLEAN-- + +--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) { + } + } + } +}