diff --git a/NEWS b/NEWS index ac3381900831..9e6a44627df4 100644 --- a/NEWS +++ b/NEWS @@ -280,6 +280,9 @@ PHP NEWS (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-- - 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" 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-- +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