[php-src] master: Fix GH-21682: reject ZipArchive serialization via __serialize() (#21708)
Ilia Alshanetsky via GitHub <[email protected]>
| Newsgroups | gmane.comp.php.cvs.general |
|---|---|
| Message-ID | <[email protected]> |
Author: Ilia Alshanetsky (iliaal)
Committer: GitHub (web-flow)
Pusher: iliaal
Date: 2026-06-26T06:31:16-04:00
Commit: https://github.com/php/php-src/commit/adc5f8da339c6a5ef80afd64c77735456f5e307d
Raw diff: https://github.com/php/php-src/commit/adc5f8da339c6a5ef80afd64c77735456f5e307d.diff
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
Changed paths:
A ext/zip/tests/bug72434.phpt
A ext/zip/tests/gh21682.phpt
A ext/zip/tests/gh21682_subclass.phpt
A ext/zip/tests/gh21682_subclass_no_overrides.phpt
D ext/standard/tests/strings/bug72434.phpt
M NEWS
M ext/zip/php_zip.c
M ext/zip/php_zip.stub.php
M ext/zip/php_zip_arginfo.h
Diff:
diff --git a/NEWS b/NEWS
index 1fe33f473af6..f13f2e00ca4d 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 6d64baa26fa7..000000000000
--- 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 12344450678b..f231b3ad9ef2 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 49dd19e53553..2cece2791d1b 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 faa6feb1cb1e..4f8366dc1512 100644
--- a/ext/zip/php_zip_arginfo.h
+++ b/ext/zip/php_zip_arginfo.h
@@ -1,5 +1,5 @@
/* This is a generated file, edit php_zip.stub.php instead.
- * Stub hash: d623efdfe5ac46f07aebf8fb120050c818f3d793 */
+ * Stub hash: 206d9be6640ee7e94d68d7e075ab61bf3188cab3 */
ZEND_BEGIN_ARG_INFO_EX(arginfo_zip_open, 0, 0, 1)
ZEND_ARG_TYPE_INFO(0, filename, IS_STRING, 0)
@@ -63,6 +63,13 @@ ZEND_END_ARG_INFO()
ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_INFO_EX(arginfo_class_ZipArchive_count, 0, 0, IS_LONG, 0)
ZEND_END_ARG_INFO()
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_ZipArchive___serialize, 0, 0, IS_ARRAY, 0)
+ZEND_END_ARG_INFO()
+
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_ZipArchive___unserialize, 0, 1, IS_VOID, 0)
+ ZEND_ARG_TYPE_INFO(0, data, IS_ARRAY, 0)
+ZEND_END_ARG_INFO()
+
ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_INFO_EX(arginfo_class_ZipArchive_getStatusString, 0, 0, IS_STRING, 0)
ZEND_END_ARG_INFO()
@@ -326,6 +333,8 @@ ZEND_METHOD(ZipArchive, setPassword);
ZEND_METHOD(ZipArchive, close);
ZEND_METHOD(ZipArchive, closeString);
ZEND_METHOD(ZipArchive, count);
+ZEND_METHOD(ZipArchive, __serialize);
+ZEND_METHOD(ZipArchive, __unserialize);
ZEND_METHOD(ZipArchive, getStatusString);
ZEND_METHOD(ZipArchive, clearError);
ZEND_METHOD(ZipArchive, addEmptyDir);
@@ -406,6 +415,8 @@ static const zend_function_entry class_ZipArchive_methods[] = {
ZEND_ME(ZipArchive, close, arginfo_class_ZipArchive_close, ZEND_ACC_PUBLIC)
ZEND_ME(ZipArchive, closeString, arginfo_class_ZipArchive_closeString, ZEND_ACC_PUBLIC)
ZEND_ME(ZipArchive, count, arginfo_class_ZipArchive_count, ZEND_ACC_PUBLIC)
+ ZEND_ME(ZipArchive, __serialize, arginfo_class_ZipArchive___serialize, ZEND_ACC_PUBLIC)
+ ZEND_ME(ZipArchive, __unserialize, arginfo_class_ZipArchive___unserialize, ZEND_ACC_PUBLIC)
ZEND_ME(ZipArchive, getStatusString, arginfo_class_ZipArchive_getStatusString, ZEND_ACC_PUBLIC)
ZEND_ME(ZipArchive, clearError, arginfo_class_ZipArchive_clearError, ZEND_ACC_PUBLIC)
ZEND_ME(ZipArchive, addEmptyDir, arginfo_class_ZipArchive_addEmptyDir, ZEND_ACC_PUBLIC)
diff --git a/ext/zip/tests/bug72434.phpt b/ext/zip/tests/bug72434.phpt
new file mode 100644
index 000000000000..e2ccebad3754
--- /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 000000000000..ee09c73e5c4d
--- /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 000000000000..8c50dd4f3a24
--- /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 000000000000..04fb49d9848e
--- /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