From e2da92b15c71d2a97420ca590fa6579f049d008a Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Wed, 27 Aug 2025 05:37:45 +0200 Subject: [PATCH] Fix ZipArchive callback being called after executor has shut down free_obj() for objects referenced in the main symbol table may be called only once the executor has already shut down. php_zip_cancel_callback() may attempt to invoke a user callback, which will terminate the process because user code is not expected to be executed at this point. We solve this by calling the callback in dtor_obj(), which is called earlier in the shutdown sequence. dtor_obj() will actually attempt to call it again if the object was reinitialized in the destructor, so we also avoid calling the callback when the executor has shut down in the first place. This should never matter in practice. Closes GH-19602 --- NEWS | 4 ++++ ext/zip/php_zip.c | 24 ++++++++++++++++++++++ ext/zip/tests/ZipArchive_bailout.phpt | 28 ++++++++++++++++++++++++++ ext/zip/tests/ZipArchive_destruct.phpt | 28 ++++++++++++++++++++++++++ 4 files changed, 84 insertions(+) create mode 100644 ext/zip/tests/ZipArchive_bailout.phpt create mode 100644 ext/zip/tests/ZipArchive_destruct.phpt diff --git a/NEWS b/NEWS index 986783ce042..eea553c1051 100644 --- a/NEWS +++ b/NEWS @@ -29,4 +29,8 @@ PHP NEWS . Fixed bug GH-19926 (reset internal pointer earlier while splicing array while COW violation flag is still set). (alexandre-daubois) +- Zip: + . Fixed ZipArchive callback being called after executor has shut down. + (ilutov) + <<< NOTE: Insert NEWS from last stable release here prior to actual release! >>> diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c index 54d2fb4ff96..cfc748ae86f 100644 --- a/ext/zip/php_zip.c +++ b/ext/zip/php_zip.c @@ -1002,6 +1002,21 @@ static void php_zip_cancel_callback_free(void *ptr) } #endif +static void php_zip_object_dtor(zend_object *object) +{ + zend_objects_destroy_object(object); + + ze_zip_object *intern = php_zip_fetch_object(object); + + if (intern->za) { + if (zip_close(intern->za) != 0) { + php_error_docref(NULL, E_WARNING, "Cannot destroy the zip context: %s", zip_strerror(intern->za)); + zip_discard(intern->za); + } + intern->za = NULL; + } +} + static void php_zip_object_free_storage(zend_object *object) /* {{{ */ { ze_zip_object * intern = php_zip_fetch_object(object); @@ -2995,6 +3010,10 @@ PHP_METHOD(ZipArchive, getStream) #ifdef HAVE_PROGRESS_CALLBACK static void php_zip_progress_callback(zip_t *arch, double state, void *ptr) { + if (!EG(active)) { + return; + } + zval cb_args[1]; ze_zip_object *obj = ptr; @@ -3041,6 +3060,10 @@ static int php_zip_cancel_callback(zip_t *arch, void *ptr) zval cb_retval; ze_zip_object *obj = ptr; + if (!EG(active)) { + return 0; + } + zend_call_known_fcc(&obj->cancel_callback, &cb_retval, 0, NULL, NULL); if (Z_ISUNDEF(cb_retval)) { /* Cancel if an exception has been thrown */ @@ -3128,6 +3151,7 @@ static PHP_MINIT_FUNCTION(zip) memcpy(&zip_object_handlers, &std_object_handlers, sizeof(zend_object_handlers)); zip_object_handlers.offset = XtOffsetOf(ze_zip_object, zo); zip_object_handlers.free_obj = php_zip_object_free_storage; + zip_object_handlers.dtor_obj = php_zip_object_dtor; zip_object_handlers.clone_obj = NULL; zip_object_handlers.get_property_ptr_ptr = php_zip_get_property_ptr_ptr; diff --git a/ext/zip/tests/ZipArchive_bailout.phpt b/ext/zip/tests/ZipArchive_bailout.phpt new file mode 100644 index 00000000000..c7e4ede8446 --- /dev/null +++ b/ext/zip/tests/ZipArchive_bailout.phpt @@ -0,0 +1,28 @@ +--TEST-- +ZipArchive destructor should be called on bailout +--EXTENSIONS-- +zip +--FILE-- +open($file, ZIPARCHIVE::CREATE); +$zip->registerCancelCallback(cb(...)); +$zip->addFromString('test', 'test'); +$fusion = $zip; + +?> +--CLEAN-- + +--EXPECTF-- +Notice: Only variable references should be returned by reference in %s on line %d + +Notice: Only variable references should be returned by reference in %s on line %d + +Notice: Only variable references should be returned by reference in %s on line %d diff --git a/ext/zip/tests/ZipArchive_destruct.phpt b/ext/zip/tests/ZipArchive_destruct.phpt new file mode 100644 index 00000000000..7e3ef6c5dae --- /dev/null +++ b/ext/zip/tests/ZipArchive_destruct.phpt @@ -0,0 +1,28 @@ +--TEST-- +Leaking ZipArchive destructor +--EXTENSIONS-- +zip +--FILE-- +open($file, ZIPARCHIVE::CREATE); +$leak->addFromString('test', 'test'); + +?> +===DONE=== +--CLEAN-- + +--EXPECT-- +===DONE===