From a1028ff49b3507558c9b81c074e0a7bdbda99309 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Sun, 14 Jun 2026 18:52:59 -0400 Subject: [PATCH] Fix GH-21682: reject ZipArchive serialization via __serialize() 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 --- ext/standard/tests/strings/bug72434.phpt | 29 --------------------- ext/zip/php_zip.c | 25 ++++++++++++++++++ ext/zip/php_zip.stub.php | 4 +++ ext/zip/php_zip_arginfo.h | 13 +++++++++- ext/zip/tests/bug72434.phpt | 17 +++++++++++++ ext/zip/tests/gh21682.phpt | 16 ++++++++++++ ext/zip/tests/gh21682_subclass.phpt | 32 ++++++++++++++++++++++++ 7 files changed, 106 insertions(+), 30 deletions(-) delete mode 100644 ext/standard/tests/strings/bug72434.phpt create mode 100644 ext/zip/tests/bug72434.phpt create mode 100644 ext/zip/tests/gh21682.phpt create mode 100644 ext/zip/tests/gh21682_subclass.phpt 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-- - 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-- +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-- +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-- + $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"