diff --git a/NEWS b/NEWS index 6c70c7891be..190eee191e8 100644 --- a/NEWS +++ b/NEWS @@ -88,6 +88,7 @@ PHP NEWS - SPL: . Fixed bug GH-17198 (SplFixedArray assertion failure with get_object_vars). (nielsdos) + . 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 diff --git a/ext/spl/spl_directory.c b/ext/spl/spl_directory.c index 94b6e0e604b..b14d182dcd7 100644 --- a/ext/spl/spl_directory.c +++ b/ext/spl/spl_directory.c @@ -203,10 +203,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(const 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) { @@ -636,7 +642,7 @@ static inline HashTable *spl_filesystem_object_get_debug_info(zend_object *objec } if (intern->type == SPL_FS_DIR) { #ifdef HAVE_GLOB - 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); @@ -1590,11 +1596,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 00000000000..66a52bce7f4 --- /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) { + } + } + } +}