Commit adc5f8da339 for php.net
commit adc5f8da339c6a5ef80afd64c77735456f5e307d
Author: Ilia Alshanetsky <ilia@ilia.ws>
Date: Fri Jun 26 06:31:16 2026 -0400
Fix GH-21682: reject ZipArchive serialization via __serialize() (#21708)
ZipArchive wraps a libzip handle that cannot survive serialization:
serialize() produced a string that unserialized into an empty object
with numFiles 0, and that unserialize path was the bug72434
use-after-free vector. Add __serialize() and __unserialize() that throw,
so the base class rejects (un)serialization and the UAF is closed by
construction, while a subclass can still override both to round-trip
through closeString()/openString(). Move the bug72434 test to
ext/zip/tests since it now requires the zip extension.
Fixes GH-21682
diff --git a/NEWS b/NEWS
index 1fe33f473af..f13f2e00ca4 100644
--- a/NEWS
+++ b/NEWS
@@ -313,6 +313,9 @@ PHP NEWS
. Added Uri\Rfc3986\UriBuilder. (kocsismate)
- Zip:
+ . Fixed bug GH-21682 (ZipArchive instances should not be serializable).
+ serialize()/unserialize() now throw unless a subclass overrides
+ __serialize()/__unserialize(). (iliaal)
. Fixed ZipArchive callback being called after executor has shut down.
(ilutov)
. Support minimum version for libzip dependency updated to 1.0.0.
diff --git a/ext/standard/tests/strings/bug72434.phpt b/ext/standard/tests/strings/bug72434.phpt
deleted file mode 100644
index 6d64baa26fa..00000000000
--- a/ext/standard/tests/strings/bug72434.phpt
+++ /dev/null
@@ -1,29 +0,0 @@
---TEST--
-Bug #72434: ZipArchive class Use After Free Vulnerability in PHP's GC algorithm and unserialize
---FILE--
-<?php
-// The following array will be serialized and this representation will be freed later on.
-$free_me = array(new StdClass());
-// Create our payload and unserialize it.
-$serialized_payload = 'a:3:{i:1;N;i:2;O:10:"ZipArchive":1:{s:8:"filename";'.serialize($free_me).'}i:1;R:4;}';
-$unserialized_payload = unserialize($serialized_payload);
-gc_collect_cycles();
-// The reference counter for $free_me is at -1 for PHP 7 right now.
-// Increment the reference counter by 1 -> rc is 0
-$a = $unserialized_payload[1];
-// Increment the reference counter by 1 again -> rc is 1
-$b = $a;
-// Trigger free of $free_me (referenced by $m[1]).
-unset($b);
-$fill_freed_space_1 = "filler_zval_1";
-$fill_freed_space_2 = "filler_zval_2";
-$fill_freed_space_3 = "filler_zval_3";
-$fill_freed_space_4 = "filler_zval_4";
-debug_zval_dump($unserialized_payload[1]);
-?>
---EXPECTF--
-array(1) refcount(3){
- [0]=>
- object(stdClass)#%d (0) refcount(1){
- }
-}
diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c
index 12344450678..f231b3ad9ef 100644
--- a/ext/zip/php_zip.c
+++ b/ext/zip/php_zip.c
@@ -25,6 +25,7 @@
#include "ext/standard/php_filestat.h"
#include "zend_attributes.h"
#include "zend_interfaces.h"
+#include "zend_exceptions.h"
#include "php_zip.h"
#include "php_zip_arginfo.h"
@@ -1645,6 +1646,30 @@ PHP_METHOD(ZipArchive, count)
}
/* }}} */
+PHP_METHOD(ZipArchive, __serialize)
+{
+ ZEND_PARSE_PARAMETERS_NONE();
+
+ zend_throw_exception_ex(NULL, 0,
+ "Serialization of '%s' is not allowed, override __serialize() and __unserialize() to implement it",
+ ZSTR_VAL(Z_OBJCE_P(ZEND_THIS)->name));
+}
+
+PHP_METHOD(ZipArchive, __unserialize)
+{
+ zval *data;
+
+ ZEND_PARSE_PARAMETERS_START(1, 1)
+ Z_PARAM_ARRAY(data)
+ ZEND_PARSE_PARAMETERS_END();
+
+ (void) data;
+
+ zend_throw_exception_ex(NULL, 0,
+ "Unserialization of '%s' is not allowed, override __serialize() and __unserialize() to implement it",
+ ZSTR_VAL(Z_OBJCE_P(ZEND_THIS)->name));
+}
+
/* {{{ clear the internal status */
PHP_METHOD(ZipArchive, clearError)
{
diff --git a/ext/zip/php_zip.stub.php b/ext/zip/php_zip.stub.php
index 49dd19e5355..2cece2791d1 100644
--- a/ext/zip/php_zip.stub.php
+++ b/ext/zip/php_zip.stub.php
@@ -661,6 +661,10 @@ public function closeString(): string|false {}
/** @tentative-return-type */
public function count(): int {}
+ public function __serialize(): array {}
+
+ public function __unserialize(array $data): void {}
+
/** @tentative-return-type */
public function getStatusString(): string {}
diff --git a/ext/zip/php_zip_arginfo.h b/ext/zip/php_zip_arginfo.h
index faa6feb1cb1..4f8366dc151 100644
Binary files a/ext/zip/php_zip_arginfo.h and b/ext/zip/php_zip_arginfo.h differ
diff --git a/ext/zip/tests/bug72434.phpt b/ext/zip/tests/bug72434.phpt
new file mode 100644
index 00000000000..e2ccebad375
--- /dev/null
+++ b/ext/zip/tests/bug72434.phpt
@@ -0,0 +1,17 @@
+--TEST--
+Bug #72434: ZipArchive class Use After Free Vulnerability in PHP's GC algorithm and unserialize
+--EXTENSIONS--
+zip
+--FILE--
+<?php
+$free_me = array(new StdClass());
+$serialized_payload = 'a:3:{i:1;N;i:2;O:10:"ZipArchive":1:{s:8:"filename";'.serialize($free_me).'}i:1;R:4;}';
+try {
+ $unserialized_payload = unserialize($serialized_payload);
+ var_dump($unserialized_payload);
+} catch (Exception $e) {
+ echo $e->getMessage() . "\n";
+}
+?>
+--EXPECT--
+Unserialization of 'ZipArchive' is not allowed, override __serialize() and __unserialize() to implement it
diff --git a/ext/zip/tests/gh21682.phpt b/ext/zip/tests/gh21682.phpt
new file mode 100644
index 00000000000..ee09c73e5c4
--- /dev/null
+++ b/ext/zip/tests/gh21682.phpt
@@ -0,0 +1,16 @@
+--TEST--
+GH-21682 (ZipArchive serialization is rejected)
+--EXTENSIONS--
+zip
+--FILE--
+<?php
+$a = new ZipArchive();
+try {
+ serialize($a);
+ echo "ERROR: should have thrown\n";
+} catch (\Exception $e) {
+ echo $e->getMessage() . "\n";
+}
+?>
+--EXPECT--
+Serialization of 'ZipArchive' is not allowed, override __serialize() and __unserialize() to implement it
diff --git a/ext/zip/tests/gh21682_subclass.phpt b/ext/zip/tests/gh21682_subclass.phpt
new file mode 100644
index 00000000000..8c50dd4f3a2
--- /dev/null
+++ b/ext/zip/tests/gh21682_subclass.phpt
@@ -0,0 +1,32 @@
+--TEST--
+GH-21682 (ZipArchive subclass implements serialization via __serialize()/__unserialize())
+--EXTENSIONS--
+zip
+--FILE--
+<?php
+class MyArchive extends ZipArchive
+{
+ public function __serialize(): array
+ {
+ return ['data' => $this->closeString()];
+ }
+
+ public function __unserialize(array $data): void
+ {
+ $this->openString($data['data']);
+ }
+}
+
+$zip = new MyArchive();
+$zip->openString();
+$zip->addFromString('test1', 'abc123');
+
+$roundtrip = unserialize(serialize($zip));
+var_dump($roundtrip instanceof MyArchive);
+var_dump($roundtrip->numFiles);
+var_dump($roundtrip->getFromName('test1'));
+?>
+--EXPECT--
+bool(true)
+int(1)
+string(6) "abc123"
diff --git a/ext/zip/tests/gh21682_subclass_no_overrides.phpt b/ext/zip/tests/gh21682_subclass_no_overrides.phpt
new file mode 100644
index 00000000000..04fb49d9848
--- /dev/null
+++ b/ext/zip/tests/gh21682_subclass_no_overrides.phpt
@@ -0,0 +1,26 @@
+--TEST--
+GH-21682 (ZipArchive subclass without overrides inherits the serialization throw)
+--EXTENSIONS--
+zip
+--FILE--
+<?php
+class MyZip extends ZipArchive {}
+
+$zip = new MyZip();
+try {
+ serialize($zip);
+ echo "ERROR: should have thrown\n";
+} catch (\Exception $e) {
+ echo $e->getMessage() . "\n";
+}
+
+try {
+ unserialize('O:5:"MyZip":0:{}');
+ echo "ERROR: should have thrown\n";
+} catch (\Exception $e) {
+ echo $e->getMessage() . "\n";
+}
+?>
+--EXPECT--
+Serialization of 'MyZip' is not allowed, override __serialize() and __unserialize() to implement it
+Unserialization of 'MyZip' is not allowed, override __serialize() and __unserialize() to implement it