Skip to content

Commit 9895509

Browse files
committed
Use dtor instead of free_obj for ZipArchive
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 using the dtor_obj handler instead, which is called earlier in the shutdown sequence. Technically, this allows some shenanigans with escaping __destruct functions, but given destructors are useful, this may not be worth solving. For reference: class MyZipArchive extends ZipArchive { public function __destruct() { global $leak; $leak = $this; } } new MyZipArchive; $file = __DIR__ . '/gh18907.zip'; $leak->open($file, ZIPARCHIVE::CREATE); $leak->addFromString('test', 'test');
1 parent e844e68 commit 9895509

File tree

2 files changed

+32
-3
lines changed

2 files changed

+32
-3
lines changed

ext/zip/php_zip.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,8 +1004,10 @@ static void php_zip_cancel_callback_free(void *ptr)
10041004
}
10051005
#endif
10061006

1007-
static void php_zip_object_free_storage(zend_object *object) /* {{{ */
1007+
static void php_zip_object_dtor(zend_object *object) /* {{{ */
10081008
{
1009+
zend_objects_destroy_object(object);
1010+
10091011
ze_zip_object * intern = php_zip_fetch_object(object);
10101012
int i;
10111013

@@ -1034,7 +1036,6 @@ static void php_zip_object_free_storage(zend_object *object) /* {{{ */
10341036
#endif
10351037

10361038
intern->za = NULL;
1037-
zend_object_std_dtor(&intern->zo);
10381039

10391040
if (intern->filename) {
10401041
efree(intern->filename);
@@ -3119,7 +3120,7 @@ static PHP_MINIT_FUNCTION(zip)
31193120
{
31203121
memcpy(&zip_object_handlers, &std_object_handlers, sizeof(zend_object_handlers));
31213122
zip_object_handlers.offset = XtOffsetOf(ze_zip_object, zo);
3122-
zip_object_handlers.free_obj = php_zip_object_free_storage;
3123+
zip_object_handlers.dtor_obj = php_zip_object_dtor;
31233124
zip_object_handlers.clone_obj = NULL;
31243125
zip_object_handlers.get_property_ptr_ptr = php_zip_get_property_ptr_ptr;
31253126

ext/zip/tests/ZipArchive_bailout.phpt

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
--TEST--
2+
ZipArchive destructor should be called on bailout
3+
--EXTENSIONS--
4+
zip
5+
--FILE--
6+
<?php
7+
8+
function &cb() {}
9+
10+
$file = __DIR__ . '/gh18907.zip';
11+
$zip = new ZipArchive;
12+
$zip->open($file, ZIPARCHIVE::CREATE);
13+
$zip->registerCancelCallback(cb(...));
14+
$zip->addFromString('test', 'test');
15+
$fusion = $zip;
16+
17+
?>
18+
--CLEAN--
19+
<?php
20+
$file = __DIR__ . '/gh18907.zip';
21+
@unlink($file);
22+
?>
23+
--EXPECTF--
24+
Notice: Only variable references should be returned by reference in %s on line %d
25+
26+
Notice: Only variable references should be returned by reference in %s on line %d
27+
28+
Notice: Only variable references should be returned by reference in %s on line %d

0 commit comments

Comments
 (0)