Skip to content

Commit a1028ff

Browse files
committed
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
1 parent 7c86d86 commit a1028ff

7 files changed

Lines changed: 106 additions & 30 deletions

File tree

ext/standard/tests/strings/bug72434.phpt

Lines changed: 0 additions & 29 deletions
This file was deleted.

ext/zip/php_zip.c

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "ext/standard/php_filestat.h"
2626
#include "zend_attributes.h"
2727
#include "zend_interfaces.h"
28+
#include "zend_exceptions.h"
2829
#include "php_zip.h"
2930
#include "php_zip_arginfo.h"
3031

@@ -1645,6 +1646,30 @@ PHP_METHOD(ZipArchive, count)
16451646
}
16461647
/* }}} */
16471648

1649+
PHP_METHOD(ZipArchive, __serialize)
1650+
{
1651+
ZEND_PARSE_PARAMETERS_NONE();
1652+
1653+
zend_throw_exception_ex(NULL, 0,
1654+
"Serialization of '%s' is not allowed, override __serialize() and __unserialize() to implement it",
1655+
ZSTR_VAL(Z_OBJCE_P(ZEND_THIS)->name));
1656+
}
1657+
1658+
PHP_METHOD(ZipArchive, __unserialize)
1659+
{
1660+
zval *data;
1661+
1662+
ZEND_PARSE_PARAMETERS_START(1, 1)
1663+
Z_PARAM_ARRAY(data)
1664+
ZEND_PARSE_PARAMETERS_END();
1665+
1666+
(void) data;
1667+
1668+
zend_throw_exception_ex(NULL, 0,
1669+
"Unserialization of '%s' is not allowed, override __serialize() and __unserialize() to implement it",
1670+
ZSTR_VAL(Z_OBJCE_P(ZEND_THIS)->name));
1671+
}
1672+
16481673
/* {{{ clear the internal status */
16491674
PHP_METHOD(ZipArchive, clearError)
16501675
{

ext/zip/php_zip.stub.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -661,6 +661,10 @@ public function closeString(): string|false {}
661661
/** @tentative-return-type */
662662
public function count(): int {}
663663

664+
public function __serialize(): array {}
665+
666+
public function __unserialize(array $data): void {}
667+
664668
/** @tentative-return-type */
665669
public function getStatusString(): string {}
666670

ext/zip/php_zip_arginfo.h

Lines changed: 12 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

ext/zip/tests/bug72434.phpt

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
Bug #72434: ZipArchive class Use After Free Vulnerability in PHP's GC algorithm and unserialize
3+
--EXTENSIONS--
4+
zip
5+
--FILE--
6+
<?php
7+
$free_me = array(new StdClass());
8+
$serialized_payload = 'a:3:{i:1;N;i:2;O:10:"ZipArchive":1:{s:8:"filename";'.serialize($free_me).'}i:1;R:4;}';
9+
try {
10+
$unserialized_payload = unserialize($serialized_payload);
11+
var_dump($unserialized_payload);
12+
} catch (Exception $e) {
13+
echo $e->getMessage() . "\n";
14+
}
15+
?>
16+
--EXPECT--
17+
Unserialization of 'ZipArchive' is not allowed, override __serialize() and __unserialize() to implement it

ext/zip/tests/gh21682.phpt

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
--TEST--
2+
GH-21682 (ZipArchive serialization is rejected)
3+
--EXTENSIONS--
4+
zip
5+
--FILE--
6+
<?php
7+
$a = new ZipArchive();
8+
try {
9+
serialize($a);
10+
echo "ERROR: should have thrown\n";
11+
} catch (\Exception $e) {
12+
echo $e->getMessage() . "\n";
13+
}
14+
?>
15+
--EXPECT--
16+
Serialization of 'ZipArchive' is not allowed, override __serialize() and __unserialize() to implement it
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
--TEST--
2+
GH-21682 (ZipArchive subclass implements serialization via __serialize()/__unserialize())
3+
--EXTENSIONS--
4+
zip
5+
--FILE--
6+
<?php
7+
class MyArchive extends ZipArchive
8+
{
9+
public function __serialize(): array
10+
{
11+
return ['data' => $this->closeString()];
12+
}
13+
14+
public function __unserialize(array $data): void
15+
{
16+
$this->openString($data['data']);
17+
}
18+
}
19+
20+
$zip = new MyArchive();
21+
$zip->openString();
22+
$zip->addFromString('test1', 'abc123');
23+
24+
$roundtrip = unserialize(serialize($zip));
25+
var_dump($roundtrip instanceof MyArchive);
26+
var_dump($roundtrip->numFiles);
27+
var_dump($roundtrip->getFromName('test1'));
28+
?>
29+
--EXPECT--
30+
bool(true)
31+
int(1)
32+
string(6) "abc123"

0 commit comments

Comments
 (0)