Skip to content

Commit 1ea12af

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 9596ab1 commit 1ea12af

2 files changed

Lines changed: 70 additions & 2 deletions

File tree

ext/zip/php_zip.c

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2910,7 +2910,10 @@ static void php_zip_progress_callback(zip_t *arch, double state, void *ptr)
29102910
ze_zip_object *obj = ptr;
29112911

29122912
ZVAL_DOUBLE(&cb_args[0], state);
2913-
zend_call_known_fcc(&obj->progress_callback, NULL, 1, cb_args, NULL);
2913+
2914+
zend_try {
2915+
zend_call_known_fcc(&obj->progress_callback, NULL, 1, cb_args, NULL);
2916+
} zend_end_try();
29142917
}
29152918

29162919
/* {{{ register a progression callback: void callback(double state); */
@@ -2957,7 +2960,13 @@ static int php_zip_cancel_callback(zip_t *arch, void *ptr)
29572960
return 0;
29582961
}
29592962

2960-
zend_call_known_fcc(&obj->cancel_callback, &cb_retval, 0, NULL, NULL);
2963+
zend_try {
2964+
zend_call_known_fcc(&obj->cancel_callback, &cb_retval, 0, NULL, NULL);
2965+
} zend_catch {
2966+
/* Cancel if a bailout occurs to allow cleanup to happen */
2967+
return -1;
2968+
} zend_end_try();
2969+
29612970
if (Z_ISUNDEF(cb_retval)) {
29622971
/* Cancel if an exception has been thrown */
29632972
return -1;

ext/zip/tests/gh22176.phpt

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
--TEST--
2+
GH-22176 (Memory leak when a ZipArchive cancel/progress 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+
if (!method_exists('ZipArchive', 'registerProgressCallback')) die('skip libzip too old');
9+
?>
10+
--FILE--
11+
<?php
12+
$dir = __DIR__ . '/';
13+
14+
$cancel = new ZipArchive;
15+
$cancel->open($dir . 'gh22176_cancel.zip', ZipArchive::CREATE);
16+
$cancel->registerCancelCallback(function () {
17+
throw new \Exception('cancel boom');
18+
});
19+
$cancel->addFromString('test', 'test');
20+
21+
$progress = new ZipArchive;
22+
$progress->open($dir . 'gh22176_progress.zip', ZipArchive::CREATE);
23+
$progress->registerProgressCallback(0.5, function ($r) {
24+
throw new \Exception('progress boom');
25+
});
26+
$progress->addFromString('test', 'test');
27+
28+
echo "done\n";
29+
// Both archives are flushed and the objects destroyed during request
30+
// shutdown; the thrown exceptions bail out through libzip's zip_close()
31+
// without leaking its internal state.
32+
?>
33+
--CLEAN--
34+
<?php
35+
@unlink(__DIR__ . '/gh22176_cancel.zip');
36+
@unlink(__DIR__ . '/gh22176_progress.zip');
37+
?>
38+
--EXPECTF--
39+
done
40+
41+
Fatal error: Uncaught Exception: progress boom in %s:%d
42+
Stack trace:
43+
#0 [internal function]: {closure:%s:%d}(0.0)
44+
#1 {main}
45+
thrown in %s on line %d
46+
47+
Fatal error: Uncaught Exception: progress boom in %s:%d
48+
Stack trace:
49+
#0 [internal function]: {closure:%s:%d}(1.0)
50+
#1 {main}
51+
thrown in %s on line %d
52+
53+
Fatal error: Uncaught Exception: cancel boom in %s:%d
54+
Stack trace:
55+
#0 [internal function]: {closure:%s:%d}()
56+
#1 {main}
57+
thrown in %s on line %d
58+
59+
Warning: PHP Request Shutdown: Cannot destroy the zip context: %s in Unknown on line 0

0 commit comments

Comments
 (0)