Skip to content

Commit c889faa

Browse files
committed
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
1 parent 2ddbc0a commit c889faa

5 files changed

Lines changed: 101 additions & 6 deletions

File tree

NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@ PHP NEWS
5555
- Zip:
5656
. Fixed bug GH-21705 (ZipArchive::getFromIndex() ignores
5757
ZipArchive::FL_UNCHANGED for deleted entries). (Weilin Du)
58+
. Fixed bug GH-22176 (memory leak with ZipArchive::registerCancelBack()
59+
is used with reference returning function during shutdown).
60+
(David Carlier)
5861

5962
02 Jul 2026, PHP 8.6.0alpha1
6063

ext/zip/php_zip.c

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -628,6 +628,12 @@ static bool php_zipobj_close(ze_zip_object *obj, zend_string **out_str) /* {{{ *
628628

629629
obj->za = NULL;
630630
obj->from_string = false;
631+
632+
if (obj->bailout_callback) {
633+
obj->bailout_callback = false;
634+
zend_bailout();
635+
}
636+
631637
return success;
632638
}
633639
/* }}} */
@@ -1073,10 +1079,16 @@ static void php_zip_object_dtor(zend_object *object)
10731079

10741080
if (intern->za) {
10751081
if (zip_close(intern->za) != 0) {
1076-
php_error_docref(NULL, E_WARNING, "Cannot destroy the zip context: %s", zip_strerror(intern->za));
1082+
if (!intern->bailout_callback) {
1083+
php_error_docref(NULL, E_WARNING, "Cannot destroy the zip context: %s", zip_strerror(intern->za));
1084+
}
10771085
zip_discard(intern->za);
10781086
}
10791087
intern->za = NULL;
1088+
if (intern->bailout_callback) {
1089+
intern->bailout_callback = false;
1090+
zend_bailout();
1091+
}
10801092
}
10811093
}
10821094

@@ -2985,15 +2997,21 @@ PHP_METHOD(ZipArchive, getStream)
29852997
#ifdef HAVE_PROGRESS_CALLBACK
29862998
static void php_zip_progress_callback(zip_t *arch, double state, void *ptr)
29872999
{
2988-
if (!EG(active)) {
3000+
ze_zip_object *obj = ptr;
3001+
3002+
if (UNEXPECTED(!EG(active) || obj->bailout_callback)) {
29893003
return;
29903004
}
29913005

29923006
zval cb_args[1];
2993-
ze_zip_object *obj = ptr;
29943007

29953008
ZVAL_DOUBLE(&cb_args[0], state);
2996-
zend_call_known_fcc(&obj->progress_callback, NULL, 1, cb_args, NULL);
3009+
3010+
zend_try {
3011+
zend_call_known_fcc(&obj->progress_callback, NULL, 1, cb_args, NULL);
3012+
} zend_catch {
3013+
obj->bailout_callback = true;
3014+
} zend_end_try();
29973015
}
29983016

29993017
/* {{{ register a progression callback: void callback(double state); */
@@ -3036,11 +3054,18 @@ static int php_zip_cancel_callback(zip_t *arch, void *ptr)
30363054
zval cb_retval;
30373055
ze_zip_object *obj = ptr;
30383056

3039-
if (!EG(active)) {
3057+
if (UNEXPECTED(!EG(active) || obj->bailout_callback)) {
30403058
return 0;
30413059
}
30423060

3043-
zend_call_known_fcc(&obj->cancel_callback, &cb_retval, 0, NULL, NULL);
3061+
zend_try {
3062+
zend_call_known_fcc(&obj->cancel_callback, &cb_retval, 0, NULL, NULL);
3063+
} zend_catch {
3064+
obj->bailout_callback = true;
3065+
/* Cancel if a bailout occurs to allow cleanup to happen */
3066+
return -1;
3067+
} zend_end_try();
3068+
30443069
if (Z_ISUNDEF(cb_retval)) {
30453070
/* Cancel if an exception has been thrown */
30463071
return -1;

ext/zip/php_zip.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ typedef struct _ze_zip_object {
7474
zip_int64_t last_id;
7575
int err_zip;
7676
int err_sys;
77+
bool bailout_callback;
7778
#ifdef HAVE_PROGRESS_CALLBACK
7879
zend_fcall_info_cache progress_callback;
7980
#endif

ext/zip/tests/gh22176.phpt

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
--TEST--
2+
GH-22176 (Memory leak when a ZipArchive cancel callback bails out in the shutdown destructor)
3+
--EXTENSIONS--
4+
zip
5+
--SKIPIF--
6+
<?php
7+
if (!method_exists('ZipArchive', 'registerCancelCallback')) die('skip libzip too old');
8+
?>
9+
--FILE--
10+
<?php
11+
$zip = new ZipArchive;
12+
$zip->open(__DIR__ . '/gh22176_cancel.zip', ZipArchive::CREATE);
13+
$zip->registerCancelCallback(function () {
14+
throw new \Exception('cancel boom');
15+
});
16+
$zip->addFromString('test', 'test');
17+
echo "done\n";
18+
// The archive is flushed and the object destroyed during request shutdown;
19+
// the thrown exception bails out through libzip's zip_close() without leaking
20+
// its internal state, and the bailout resumes once libzip has unwound.
21+
?>
22+
--CLEAN--
23+
<?php
24+
@unlink(__DIR__ . '/gh22176_cancel.zip');
25+
?>
26+
--EXPECTF--
27+
done
28+
29+
Fatal error: Uncaught Exception: cancel boom in %s:%d
30+
Stack trace:
31+
#0 [internal function]: {closure:%s:%d}()
32+
#1 {main}
33+
thrown in %s on line %d
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
--TEST--
2+
GH-22176 (Memory leak when a ZipArchive progress callback bails out in the shutdown destructor)
3+
--EXTENSIONS--
4+
zip
5+
--SKIPIF--
6+
<?php
7+
if (!method_exists('ZipArchive', 'registerProgressCallback')) die('skip libzip too old');
8+
?>
9+
--FILE--
10+
<?php
11+
$zip = new ZipArchive;
12+
$zip->open(__DIR__ . '/gh22176_progress.zip', ZipArchive::CREATE);
13+
$zip->registerProgressCallback(0.5, function ($r) {
14+
throw new \Exception('progress boom');
15+
});
16+
$zip->addFromString('test', 'test');
17+
echo "done\n";
18+
// The archive is flushed and the object destroyed during request shutdown;
19+
// the thrown exception bails out through libzip's zip_close() without leaking
20+
// its internal state, and the bailout resumes once libzip has unwound.
21+
?>
22+
--CLEAN--
23+
<?php
24+
@unlink(__DIR__ . '/gh22176_progress.zip');
25+
?>
26+
--EXPECTF--
27+
done
28+
29+
Fatal error: Uncaught Exception: progress boom in %s:%d
30+
Stack trace:
31+
#0 [internal function]: {closure:%s:%d}(0.0)
32+
#1 {main}
33+
thrown in %s on line %d

0 commit comments

Comments
 (0)