Skip to content

Commit bd5ef5a

Browse files
committed
Fix GH-9618: drain __wakeup/__unserialize queue on unserialize failure
unserialize() defers __wakeup and __unserialize until parsing ends and drains them in PHP_VAR_UNSERIALIZE_DESTROY, but the failure path runs zval_ptr_dtor(return_value) first, so a destructor of the partially-built value can reach a sibling object still in its pre-wakeup state and bypass whatever invariant that wakeup installs. Drain before that dtor. The var dtor hash is shared with nested unserialize() calls and is append-only, so the drain starts from the tail position recorded before parsing and covers only the entries this call queued. Fixes GH-9618 Closes GH-21716
1 parent 7311ab6 commit bd5ef5a

7 files changed

Lines changed: 284 additions & 59 deletions

File tree

ext/standard/php_var.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ PHPAPI void php_unserialize_with_options(zval *return_value, const char *buf, co
6262
PHPAPI void var_replace(php_unserialize_data_t *var_hash, zval *ozval, zval *nzval);
6363
PHPAPI void var_push_dtor(php_unserialize_data_t *var_hash, zval *val);
6464
PHPAPI zval *var_tmp_var(php_unserialize_data_t *var_hashx);
65+
PHPAPI zend_long var_delayed_calls_mark(php_unserialize_data_t *var_hash);
66+
PHPAPI void var_invoke_delayed_calls(php_unserialize_data_t *var_hash, zend_long from);
6567
PHPAPI void var_destroy(php_unserialize_data_t *var_hash);
6668

6769
#endif /* PHP_VAR_H */
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
--TEST--
2+
GH-9618 (unserialize __wakeup bypass via malformed payload)
3+
--FILE--
4+
<?php
5+
class A
6+
{
7+
public $info;
8+
9+
public function __destruct()
10+
{
11+
if (is_object($this->info)) {
12+
$this->info->probe();
13+
}
14+
}
15+
}
16+
17+
class B
18+
{
19+
public $end;
20+
21+
public function __wakeup()
22+
{
23+
$this->end = 'wakeup-guard';
24+
echo "B::__wakeup\n";
25+
}
26+
27+
public function __call($method, $args)
28+
{
29+
echo "B::__call end=" . var_export($this->end, true) . "\n";
30+
}
31+
}
32+
33+
$payload = 'O:1:"A":2:{s:4:"info";O:1:"B":1:{s:3:"end";N;}s:6:"Aend";s:1:"1";}';
34+
35+
var_dump(@unserialize($payload));
36+
?>
37+
--EXPECT--
38+
B::__wakeup
39+
B::__call end='wakeup-guard'
40+
bool(false)
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
--TEST--
2+
GH-9618 (a failing nested unserialize() must not drain the outer call's queue)
3+
--FILE--
4+
<?php
5+
class A
6+
{
7+
public function __wakeup()
8+
{
9+
echo "A::__wakeup\n";
10+
}
11+
}
12+
13+
class B implements Serializable
14+
{
15+
public function serialize(): string
16+
{
17+
return 'x';
18+
}
19+
20+
public function unserialize($data): void
21+
{
22+
@unserialize('O:1:"X":{BAD');
23+
echo "B::unserialize done\n";
24+
}
25+
26+
public function __serialize(): array
27+
{
28+
return [];
29+
}
30+
31+
public function __unserialize(array $data): void
32+
{
33+
}
34+
}
35+
36+
$payload = 'a:2:{i:0;O:1:"A":0:{}i:1;C:1:"B":1:{x}}';
37+
38+
var_dump(@unserialize($payload) !== false);
39+
?>
40+
--EXPECT--
41+
B::unserialize done
42+
A::__wakeup
43+
bool(true)
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
--TEST--
2+
GH-9618 (__wakeup runs before sibling destructors when the failure is nested)
3+
--FILE--
4+
<?php
5+
class A
6+
{
7+
public $info;
8+
9+
public function __destruct()
10+
{
11+
if (is_object($this->info)) {
12+
$this->info->probe();
13+
}
14+
}
15+
}
16+
17+
class B
18+
{
19+
public $end;
20+
21+
public function __wakeup()
22+
{
23+
$this->end = 'wakeup-guard';
24+
echo "B::__wakeup\n";
25+
}
26+
27+
public function __call($method, $args)
28+
{
29+
echo "B::__call end=" . var_export($this->end, true) . "\n";
30+
}
31+
}
32+
33+
class W implements Serializable
34+
{
35+
public function serialize(): string
36+
{
37+
return 'x';
38+
}
39+
40+
public function unserialize($data): void
41+
{
42+
@unserialize('O:1:"A":2:{s:4:"info";O:1:"B":1:{s:3:"end";N;}s:6:"Aend";s:1:"1";}');
43+
echo "W::unserialize done\n";
44+
}
45+
46+
public function __serialize(): array
47+
{
48+
return [];
49+
}
50+
51+
public function __unserialize(array $data): void
52+
{
53+
}
54+
}
55+
56+
var_dump(@unserialize('C:1:"W":1:{x}') !== false);
57+
?>
58+
--EXPECT--
59+
B::__wakeup
60+
W::unserialize done
61+
B::__call end='wakeup-guard'
62+
bool(true)
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
--TEST--
2+
GH-9618 (__unserialize drained before destructors on failure path)
3+
--FILE--
4+
<?php
5+
class A
6+
{
7+
public $info;
8+
9+
public function __destruct()
10+
{
11+
if (is_object($this->info)) {
12+
$this->info->probe();
13+
}
14+
}
15+
}
16+
17+
class C
18+
{
19+
public $end;
20+
21+
public function __unserialize(array $data): void
22+
{
23+
$this->end = 'unserialize-guard';
24+
echo "C::__unserialize\n";
25+
}
26+
27+
public function __serialize(): array
28+
{
29+
return ['end' => $this->end];
30+
}
31+
32+
public function __call($method, $args)
33+
{
34+
echo "C::__call end=" . var_export($this->end, true) . "\n";
35+
}
36+
}
37+
38+
$payload = 'O:1:"A":2:{s:4:"info";O:1:"C":1:{s:3:"end";N;}s:6:"Aend";s:1:"1";}';
39+
40+
var_dump(@unserialize($payload));
41+
?>
42+
--EXPECT--
43+
C::__unserialize
44+
C::__call end='unserialize-guard'
45+
bool(false)

ext/standard/var.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1409,7 +1409,7 @@ PHPAPI void php_unserialize_with_options(zval *return_value, const char *buf, co
14091409
php_unserialize_data_t var_hash;
14101410
zval *retval;
14111411
HashTable *class_hash = NULL, *prev_class_hash;
1412-
zend_long prev_max_depth, prev_cur_depth;
1412+
zend_long prev_max_depth, prev_cur_depth, delayed_calls_mark;
14131413

14141414
if (buf_len == 0) {
14151415
RETURN_FALSE;
@@ -1480,6 +1480,7 @@ PHPAPI void php_unserialize_with_options(zval *return_value, const char *buf, co
14801480
}
14811481
}
14821482

1483+
delayed_calls_mark = var_delayed_calls_mark(&var_hash);
14831484
if (BG(unserialize).level > 1) {
14841485
retval = var_tmp_var(&var_hash);
14851486
} else {
@@ -1490,6 +1491,7 @@ PHPAPI void php_unserialize_with_options(zval *return_value, const char *buf, co
14901491
php_error_docref(NULL, E_WARNING, "Error at offset " ZEND_LONG_FMT " of %zd bytes",
14911492
(zend_long)((char*)p - buf), buf_len);
14921493
}
1494+
var_invoke_delayed_calls(&var_hash, delayed_calls_mark);
14931495
if (BG(unserialize).level <= 1) {
14941496
zval_ptr_dtor(return_value);
14951497
}

0 commit comments

Comments
 (0)