From 1ea12af7929ba8400616886118a5c8a2ba92ba6a Mon Sep 17 00:00:00 2001 From: David Carlier Date: Fri, 29 May 2026 07:00:54 +0100 Subject: [PATCH 1/2] ext/zip: memory leak when zip cancel callback bails out. Fix #22176 A cancel callback that throws during the implicit zip_close() in the shutdown destructor triggers a zend_bailout that longjmps through libzip, skipping its free(filelist). Wrap the call in zend_try/zend_catch and cancel on bailout so libzip can unwind and clean up. While at it, apply the same guard to the progress callback, which is invoked from libzip the same way and is prone to the identical leak. close GH-22177 --- ext/zip/php_zip.c | 13 +++++++-- ext/zip/tests/gh22176.phpt | 59 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) create mode 100644 ext/zip/tests/gh22176.phpt diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c index a8289a41a304..faeefa897308 100644 --- a/ext/zip/php_zip.c +++ b/ext/zip/php_zip.c @@ -2910,7 +2910,10 @@ static void php_zip_progress_callback(zip_t *arch, double state, void *ptr) ze_zip_object *obj = ptr; ZVAL_DOUBLE(&cb_args[0], state); - zend_call_known_fcc(&obj->progress_callback, NULL, 1, cb_args, NULL); + + zend_try { + zend_call_known_fcc(&obj->progress_callback, NULL, 1, cb_args, NULL); + } zend_end_try(); } /* {{{ register a progression callback: void callback(double state); */ @@ -2957,7 +2960,13 @@ static int php_zip_cancel_callback(zip_t *arch, void *ptr) return 0; } - zend_call_known_fcc(&obj->cancel_callback, &cb_retval, 0, NULL, NULL); + zend_try { + zend_call_known_fcc(&obj->cancel_callback, &cb_retval, 0, NULL, NULL); + } zend_catch { + /* Cancel if a bailout occurs to allow cleanup to happen */ + return -1; + } zend_end_try(); + if (Z_ISUNDEF(cb_retval)) { /* Cancel if an exception has been thrown */ return -1; diff --git a/ext/zip/tests/gh22176.phpt b/ext/zip/tests/gh22176.phpt new file mode 100644 index 000000000000..5cc8b640a9ff --- /dev/null +++ b/ext/zip/tests/gh22176.phpt @@ -0,0 +1,59 @@ +--TEST-- +GH-22176 (Memory leak when a ZipArchive cancel/progress callback bails out in the shutdown destructor) +--EXTENSIONS-- +zip +--SKIPIF-- + +--FILE-- +open($dir . 'gh22176_cancel.zip', ZipArchive::CREATE); +$cancel->registerCancelCallback(function () { + throw new \Exception('cancel boom'); +}); +$cancel->addFromString('test', 'test'); + +$progress = new ZipArchive; +$progress->open($dir . 'gh22176_progress.zip', ZipArchive::CREATE); +$progress->registerProgressCallback(0.5, function ($r) { + throw new \Exception('progress boom'); +}); +$progress->addFromString('test', 'test'); + +echo "done\n"; +// Both archives are flushed and the objects destroyed during request +// shutdown; the thrown exceptions bail out through libzip's zip_close() +// without leaking its internal state. +?> +--CLEAN-- + +--EXPECTF-- +done + +Fatal error: Uncaught Exception: progress boom in %s:%d +Stack trace: +#0 [internal function]: {closure:%s:%d}(0.0) +#1 {main} + thrown in %s on line %d + +Fatal error: Uncaught Exception: progress boom in %s:%d +Stack trace: +#0 [internal function]: {closure:%s:%d}(1.0) +#1 {main} + thrown in %s on line %d + +Fatal error: Uncaught Exception: cancel boom in %s:%d +Stack trace: +#0 [internal function]: {closure:%s:%d}() +#1 {main} + thrown in %s on line %d + +Warning: PHP Request Shutdown: Cannot destroy the zip context: %s in Unknown on line 0 From 39e4f0e95bb5714077aa2bd2c5aa2e079d779131 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Fri, 29 May 2026 10:52:02 +0100 Subject: [PATCH 2/2] feedback then now we have to exercise the two cases separately. --- ext/zip/php_zip.c | 24 ++++++++++++++--- ext/zip/php_zip.h | 1 + ext/zip/tests/gh22176.phpt | 42 ++++++----------------------- ext/zip/tests/gh22176_progress.phpt | 33 +++++++++++++++++++++++ 4 files changed, 62 insertions(+), 38 deletions(-) create mode 100644 ext/zip/tests/gh22176_progress.phpt diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c index faeefa897308..a5bfa7b5d9f6 100644 --- a/ext/zip/php_zip.c +++ b/ext/zip/php_zip.c @@ -607,6 +607,12 @@ static bool php_zipobj_close(ze_zip_object *obj) /* {{{ */ } obj->za = NULL; + + if (obj->bailout_callback) { + obj->bailout_callback = false; + zend_bailout(); + } + return success; } /* }}} */ @@ -1049,10 +1055,16 @@ static void php_zip_object_dtor(zend_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)); + if (!intern->bailout_callback) { + php_error_docref(NULL, E_WARNING, "Cannot destroy the zip context: %s", zip_strerror(intern->za)); + } zip_discard(intern->za); } intern->za = NULL; + if (intern->bailout_callback) { + intern->bailout_callback = false; + zend_bailout(); + } } } @@ -2902,17 +2914,20 @@ PHP_METHOD(ZipArchive, getStream) #ifdef HAVE_PROGRESS_CALLBACK static void php_zip_progress_callback(zip_t *arch, double state, void *ptr) { - if (!EG(active)) { + ze_zip_object *obj = ptr; + + if (!EG(active) || obj->bailout_callback) { return; } zval cb_args[1]; - ze_zip_object *obj = ptr; ZVAL_DOUBLE(&cb_args[0], state); zend_try { zend_call_known_fcc(&obj->progress_callback, NULL, 1, cb_args, NULL); + } zend_catch { + obj->bailout_callback = true; } zend_end_try(); } @@ -2956,13 +2971,14 @@ static int php_zip_cancel_callback(zip_t *arch, void *ptr) zval cb_retval; ze_zip_object *obj = ptr; - if (!EG(active)) { + if (!EG(active) || obj->bailout_callback) { return 0; } zend_try { zend_call_known_fcc(&obj->cancel_callback, &cb_retval, 0, NULL, NULL); } zend_catch { + obj->bailout_callback = true; /* Cancel if a bailout occurs to allow cleanup to happen */ return -1; } zend_end_try(); diff --git a/ext/zip/php_zip.h b/ext/zip/php_zip.h index cdd72ba36367..97f70c0873ad 100644 --- a/ext/zip/php_zip.h +++ b/ext/zip/php_zip.h @@ -72,6 +72,7 @@ typedef struct _ze_zip_object { zip_int64_t last_id; int err_zip; int err_sys; + bool bailout_callback; #ifdef HAVE_PROGRESS_CALLBACK zend_fcall_info_cache progress_callback; #endif diff --git a/ext/zip/tests/gh22176.phpt b/ext/zip/tests/gh22176.phpt index 5cc8b640a9ff..90b6327072c9 100644 --- a/ext/zip/tests/gh22176.phpt +++ b/ext/zip/tests/gh22176.phpt @@ -1,59 +1,33 @@ --TEST-- -GH-22176 (Memory leak when a ZipArchive cancel/progress callback bails out in the shutdown destructor) +GH-22176 (Memory leak when a ZipArchive cancel callback bails out in the shutdown destructor) --EXTENSIONS-- zip --SKIPIF-- --FILE-- open($dir . 'gh22176_cancel.zip', ZipArchive::CREATE); -$cancel->registerCancelCallback(function () { +$zip = new ZipArchive; +$zip->open(__DIR__ . '/gh22176_cancel.zip', ZipArchive::CREATE); +$zip->registerCancelCallback(function () { throw new \Exception('cancel boom'); }); -$cancel->addFromString('test', 'test'); - -$progress = new ZipArchive; -$progress->open($dir . 'gh22176_progress.zip', ZipArchive::CREATE); -$progress->registerProgressCallback(0.5, function ($r) { - throw new \Exception('progress boom'); -}); -$progress->addFromString('test', 'test'); - +$zip->addFromString('test', 'test'); echo "done\n"; -// Both archives are flushed and the objects destroyed during request -// shutdown; the thrown exceptions bail out through libzip's zip_close() -// without leaking its internal state. +// The archive is flushed and the object destroyed during request shutdown; +// the thrown exception bails out through libzip's zip_close() without leaking +// its internal state, and the bailout resumes once libzip has unwound. ?> --CLEAN-- --EXPECTF-- done -Fatal error: Uncaught Exception: progress boom in %s:%d -Stack trace: -#0 [internal function]: {closure:%s:%d}(0.0) -#1 {main} - thrown in %s on line %d - -Fatal error: Uncaught Exception: progress boom in %s:%d -Stack trace: -#0 [internal function]: {closure:%s:%d}(1.0) -#1 {main} - thrown in %s on line %d - Fatal error: Uncaught Exception: cancel boom in %s:%d Stack trace: #0 [internal function]: {closure:%s:%d}() #1 {main} thrown in %s on line %d - -Warning: PHP Request Shutdown: Cannot destroy the zip context: %s in Unknown on line 0 diff --git a/ext/zip/tests/gh22176_progress.phpt b/ext/zip/tests/gh22176_progress.phpt new file mode 100644 index 000000000000..5507fe24ab98 --- /dev/null +++ b/ext/zip/tests/gh22176_progress.phpt @@ -0,0 +1,33 @@ +--TEST-- +GH-22176 (Memory leak when a ZipArchive progress callback bails out in the shutdown destructor) +--EXTENSIONS-- +zip +--SKIPIF-- + +--FILE-- +open(__DIR__ . '/gh22176_progress.zip', ZipArchive::CREATE); +$zip->registerProgressCallback(0.5, function ($r) { + throw new \Exception('progress boom'); +}); +$zip->addFromString('test', 'test'); +echo "done\n"; +// The archive is flushed and the object destroyed during request shutdown; +// the thrown exception bails out through libzip's zip_close() without leaking +// its internal state, and the bailout resumes once libzip has unwound. +?> +--CLEAN-- + +--EXPECTF-- +done + +Fatal error: Uncaught Exception: progress boom in %s:%d +Stack trace: +#0 [internal function]: {closure:%s:%d}(0.0) +#1 {main} + thrown in %s on line %d