ext/zip: fix use-after-free in the archive destructor path

ext/zip/php_zip.c

@@ -659,7 +659,6 @@ static bool php_zipobj_close(ze_zip_object *obj, zend_string **out_str) /* {{{ *
 	if (intern) {
 		archive->close = true;
 		int err = zip_close(intern);
-		archive->close = false;
 		if (err) {
 			php_error_docref(NULL, E_WARNING, "%s", zip_strerror(intern));
 			/* Save error for property reader */
@@ -698,6 +697,7 @@ static bool php_zipobj_close(ze_zip_object *obj, zend_string **out_str) /* {{{ *
 
 	if (archive) {
 		archive->za = NULL;
+		archive->close = false;
 		bailout = archive->bailout_callback;
 		archive->bailout_callback = false;
 		obj->archive = NULL;
@@ -1128,6 +1128,10 @@ static void php_zip_progress_callback_free(void *ptr)
 {
 	php_zip_archive *archive = ptr;
 
+	if (UNEXPECTED(!EG(active) || archive->bailout_callback)) {
+		return;
+	}
+
 	if (ZEND_FCC_INITIALIZED(archive->progress_callback)) {
 		zend_fcc_dtor(&archive->progress_callback);
 	}
@@ -1139,6 +1143,10 @@ static void php_zip_cancel_callback_free(void *ptr)
 {
 	php_zip_archive *archive = ptr;
 
+	if (UNEXPECTED(!EG(active) || archive->bailout_callback)) {
+		return;
+	}
+
 	if (ZEND_FCC_INITIALIZED(archive->cancel_callback)) {
 		zend_fcc_dtor(&archive->cancel_callback);
 	}
@@ -1169,7 +1177,12 @@ bool php_zip_archive_release(php_zip_archive *archive)
 	}
 
 	if (archive->za) {
-		if (zip_close(archive->za) != 0) {
+		/* Guard against a re-entrant close() or open() from a progress/cancel
+		 * callback fired during zip_close(), which would run a nested zip_close()
+		 * on the same archive (see php_zipobj_close()). */
+		archive->close = true;
+		int err = zip_close(archive->za);
+		if (err != 0) {
 			if (!archive->bailout_callback) {
 				php_error_docref(NULL, E_WARNING, "Cannot destroy the zip context: %s", zip_strerror(archive->za));
 			}
@@ -3131,6 +3144,10 @@ static void php_zip_get_stream(INTERNAL_FUNCTION_PARAMETERS, int type, bool acce
 
 	ZIP_FROM_OBJECT(intern, self);
 
+	if (php_zipobj_closing(Z_ZIP_P(self))) {
+		RETURN_THROWS();
+	}
+
 	if (type) {
 		PHP_ZIP_STAT_PATH(intern, ZSTR_VAL(filename), ZSTR_LEN(filename), flags, sb);
 	} else {

ext/zip/tests/gh23747_close_error.phpt

@@ -0,0 +1,48 @@
+--TEST--
+GH-23747 (Re-entrant close from a ZipArchive close warning is rejected)
+--EXTENSIONS--
+zip
+--FILE--
+<?php
+$filename = __DIR__ . '/gh23747_close_error.zip';
+$source = __DIR__ . '/gh23747_close_error.txt';
+
+foreach (['close', 'destruct'] as $operation) {
+    echo $operation, ":\n";
+    file_put_contents($source, 'contents');
+    $zip = new ZipArchive();
+    $zip->open($filename, ZipArchive::CREATE | ZipArchive::OVERWRITE);
+    $zip->addFile($source, 'file.txt');
+    unlink($source);
+
+    $weak = WeakReference::create($zip);
+    set_error_handler(static function (int $errno, string $message) use ($weak): bool {
+        try {
+            $weak->get()->close();
+        } catch (Error $error) {
+            echo $error::class, ': ', $error->getMessage(), "\n";
+        }
+        return true;
+    });
+
+    if ($operation === 'close') {
+        var_dump($zip->close());
+    } else {
+        unset($zip);
+        echo "destroyed\n";
+    }
+    restore_error_handler();
+}
+?>
+--CLEAN--
+<?php
+@unlink(__DIR__ . '/gh23747_close_error.zip');
+@unlink(__DIR__ . '/gh23747_close_error.txt');
+?>
+--EXPECT--
+close:
+Error: Already being closed
+bool(false)
+destruct:
+Error: Already being closed
+destroyed

ext/zip/tests/gh23747_dtor.phpt

@@ -0,0 +1,58 @@
+--TEST--
+GH-23747 (Re-entrant operations during ZipArchive destruction are rejected)
+--EXTENSIONS--
+zip
+--SKIPIF--
+<?php
+if (!method_exists(ZipArchive::class, 'registerProgressCallback')) {
+    die('skip progress callbacks are not supported');
+}
+?>
+--FILE--
+<?php
+$filename = __DIR__ . '/gh23747_dtor.zip';
+
+foreach ([
+    ['close', []],
+    ['getStream', ['f0.txt']],
+    ['getStreamName', ['f0.txt']],
+    ['getStreamIndex', [0]],
+] as [$method, $arguments]) {
+    $zip = new ZipArchive();
+    $zip->open($filename, ZipArchive::CREATE | ZipArchive::OVERWRITE);
+    for ($index = 0; $index < 64; $index++) {
+        $zip->addFromString("f$index.txt", str_repeat('x', 2000));
+    }
+
+    $weak = WeakReference::create($zip);
+    $callback = static function (float $rate) use ($weak, $method, $arguments): void {
+        static $done = false;
+        if ($done) {
+            return;
+        }
+        $done = true;
+
+        try {
+            $weak->get()->$method(...$arguments);
+        } catch (Error $error) {
+            echo $method, ': ', $error->getMessage(), "\n";
+        }
+    };
+    $zip->registerProgressCallback(0.0, $callback);
+    unset($zip);
+    echo "destroyed\n";
+}
+?>
+--CLEAN--
+<?php
+@unlink(__DIR__ . '/gh23747_dtor.zip');
+?>
+--EXPECT--
+close: Already being closed
+destroyed
+getStream: Already being closed
+destroyed
+getStreamName: Already being closed
+destroyed
+getStreamIndex: Already being closed
+destroyed