Skip to content

Commit eb34063

Browse files
committed
Merge branch 'PHP-8.5'
* PHP-8.5: Fix GH-21927: Use-after-free of self-freeing MultipleIterator children.
2 parents 241767c + a8f1cef commit eb34063

2 files changed

Lines changed: 171 additions & 3 deletions

File tree

ext/spl/spl_observer.c

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1300,7 +1300,9 @@ PHP_METHOD(MultipleIterator, rewind)
13001300
zend_hash_internal_pointer_reset_ex(&intern->storage, &intern->pos);
13011301
while ((element = zend_hash_get_current_data_ptr_ex(&intern->storage, &intern->pos)) != NULL && !EG(exception)) {
13021302
zend_object *it = element->obj;
1303+
GC_ADDREF(it);
13031304
zend_call_known_instance_method_with_0_params(it->ce->iterator_funcs_ptr->zf_rewind, it, NULL);
1305+
OBJ_RELEASE(it);
13041306
zend_hash_move_forward_ex(&intern->storage, &intern->pos);
13051307
}
13061308
}
@@ -1319,7 +1321,9 @@ PHP_METHOD(MultipleIterator, next)
13191321
zend_hash_internal_pointer_reset_ex(&intern->storage, &intern->pos);
13201322
while ((element = zend_hash_get_current_data_ptr_ex(&intern->storage, &intern->pos)) != NULL && !EG(exception)) {
13211323
zend_object *it = element->obj;
1324+
GC_ADDREF(it);
13221325
zend_call_known_instance_method_with_0_params(it->ce->iterator_funcs_ptr->zf_next, it, NULL);
1326+
OBJ_RELEASE(it);
13231327
zend_hash_move_forward_ex(&intern->storage, &intern->pos);
13241328
}
13251329
}
@@ -1346,7 +1350,9 @@ PHP_METHOD(MultipleIterator, valid)
13461350
zend_hash_internal_pointer_reset_ex(&intern->storage, &intern->pos);
13471351
while ((element = zend_hash_get_current_data_ptr_ex(&intern->storage, &intern->pos)) != NULL && !EG(exception)) {
13481352
zend_object *it = element->obj;
1353+
GC_ADDREF(it);
13491354
zend_call_known_instance_method_with_0_params(it->ce->iterator_funcs_ptr->zf_valid, it, &retval);
1355+
OBJ_RELEASE(it);
13501356

13511357
if (!Z_ISUNDEF(retval)) {
13521358
valid = (Z_TYPE(retval) == IS_TRUE);
@@ -1384,6 +1390,9 @@ static void spl_multiple_iterator_get_all(spl_SplObjectStorage *intern, int get_
13841390
zend_hash_internal_pointer_reset_ex(&intern->storage, &intern->pos);
13851391
while ((element = zend_hash_get_current_data_ptr_ex(&intern->storage, &intern->pos)) != NULL && !EG(exception)) {
13861392
zend_object *it = element->obj;
1393+
zval inf;
1394+
GC_ADDREF(it);
1395+
ZVAL_COPY(&inf, &element->inf);
13871396
zend_call_known_instance_method_with_0_params(it->ce->iterator_funcs_ptr->zf_valid, it, &retval);
13881397

13891398
if (!Z_ISUNDEF(retval)) {
@@ -1400,10 +1409,14 @@ static void spl_multiple_iterator_get_all(spl_SplObjectStorage *intern, int get_
14001409
zend_call_known_instance_method_with_0_params(it->ce->iterator_funcs_ptr->zf_key, it, &retval);
14011410
}
14021411
if (Z_ISUNDEF(retval)) {
1412+
OBJ_RELEASE(it);
1413+
zval_ptr_dtor(&inf);
14031414
zend_throw_exception(spl_ce_RuntimeException, "Failed to call sub iterator method", 0);
14041415
return;
14051416
}
14061417
} else if (intern->flags & MIT_NEED_ALL) {
1418+
OBJ_RELEASE(it);
1419+
zval_ptr_dtor(&inf);
14071420
if (SPL_MULTIPLE_ITERATOR_GET_ALL_CURRENT == get_type) {
14081421
zend_throw_exception(spl_ce_RuntimeException, "Called current() with non valid sub iterator", 0);
14091422
} else {
@@ -1415,22 +1428,26 @@ static void spl_multiple_iterator_get_all(spl_SplObjectStorage *intern, int get_
14151428
}
14161429

14171430
if (intern->flags & MIT_KEYS_ASSOC) {
1418-
switch (Z_TYPE(element->inf)) {
1431+
switch (Z_TYPE(inf)) {
14191432
case IS_LONG:
1420-
add_index_zval(return_value, Z_LVAL(element->inf), &retval);
1433+
add_index_zval(return_value, Z_LVAL(inf), &retval);
14211434
break;
14221435
case IS_STRING:
1423-
zend_symtable_update(Z_ARRVAL_P(return_value), Z_STR(element->inf), &retval);
1436+
zend_symtable_update(Z_ARRVAL_P(return_value), Z_STR(inf), &retval);
14241437
break;
14251438
default:
14261439
zval_ptr_dtor(&retval);
1440+
OBJ_RELEASE(it);
1441+
zval_ptr_dtor(&inf);
14271442
zend_throw_exception(spl_ce_InvalidArgumentException, "Sub-Iterator is associated with NULL", 0);
14281443
return;
14291444
}
14301445
} else {
14311446
add_next_index_zval(return_value, &retval);
14321447
}
14331448

1449+
OBJ_RELEASE(it);
1450+
zval_ptr_dtor(&inf);
14341451
zend_hash_move_forward_ex(&intern->storage, &intern->pos);
14351452
}
14361453
}

ext/spl/tests/gh21927.phpt

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
--TEST--
2+
GH-21927: Use-after-free of self-freeing MultipleIterator children
3+
--FILE--
4+
<?php
5+
6+
class DetachOnRewind implements Iterator {
7+
public function __construct(private MultipleIterator $parent) {}
8+
public function rewind(): void {
9+
$this->parent->detachIterator($this);
10+
echo "rewind: still alive\n";
11+
}
12+
public function next(): void {}
13+
public function current(): mixed { return 0; }
14+
public function key(): mixed { return 0; }
15+
public function valid(): bool { return false; }
16+
}
17+
18+
class DetachOnNext implements Iterator {
19+
public function __construct(private MultipleIterator $parent) {}
20+
public function rewind(): void {}
21+
public function next(): void {
22+
$this->parent->detachIterator($this);
23+
echo "next: still alive\n";
24+
}
25+
public function current(): mixed { return 0; }
26+
public function key(): mixed { return 0; }
27+
public function valid(): bool { return true; }
28+
}
29+
30+
class DetachOnValid implements Iterator {
31+
public function __construct(private MultipleIterator $parent) {}
32+
public function rewind(): void {}
33+
public function next(): void {}
34+
public function current(): mixed { return 0; }
35+
public function key(): mixed { return 0; }
36+
public function valid(): bool {
37+
$this->parent->detachIterator($this);
38+
echo "valid: still alive\n";
39+
return true;
40+
}
41+
}
42+
43+
class DetachOnCurrent implements Iterator {
44+
public function __construct(private MultipleIterator $parent) {}
45+
public function rewind(): void {}
46+
public function next(): void {}
47+
public function current(): mixed {
48+
$this->parent->detachIterator($this);
49+
echo "current: still alive\n";
50+
return 'C';
51+
}
52+
public function key(): mixed { return 'k'; }
53+
public function valid(): bool { return true; }
54+
}
55+
56+
class DetachOnKey implements Iterator {
57+
public function __construct(private MultipleIterator $parent) {}
58+
public function rewind(): void {}
59+
public function next(): void {}
60+
public function current(): mixed { return 'C'; }
61+
public function key(): mixed {
62+
$this->parent->detachIterator($this);
63+
echo "key: still alive\n";
64+
return 'K';
65+
}
66+
public function valid(): bool { return true; }
67+
}
68+
69+
echo "-- detach inside rewind --\n";
70+
$mi = new MultipleIterator();
71+
$mi->attachIterator(new DetachOnRewind($mi));
72+
$mi->rewind();
73+
var_dump($mi->countIterators());
74+
75+
echo "-- detach inside next --\n";
76+
$mi = new MultipleIterator();
77+
$mi->attachIterator(new DetachOnNext($mi));
78+
$mi->rewind();
79+
$mi->next();
80+
var_dump($mi->countIterators());
81+
82+
echo "-- detach inside valid --\n";
83+
$mi = new MultipleIterator();
84+
$mi->attachIterator(new DetachOnValid($mi));
85+
var_dump($mi->valid());
86+
var_dump($mi->countIterators());
87+
88+
echo "-- detach inside current (numeric keys) --\n";
89+
$mi = new MultipleIterator();
90+
$mi->attachIterator(new DetachOnCurrent($mi));
91+
var_dump($mi->current());
92+
var_dump($mi->countIterators());
93+
94+
echo "-- detach inside key (numeric keys) --\n";
95+
$mi = new MultipleIterator();
96+
$mi->attachIterator(new DetachOnKey($mi));
97+
var_dump($mi->key());
98+
var_dump($mi->countIterators());
99+
100+
echo "-- detach inside current (assoc keys, refcounted inf) --\n";
101+
$mi = new MultipleIterator(MultipleIterator::MIT_NEED_ALL | MultipleIterator::MIT_KEYS_ASSOC);
102+
$mi->attachIterator(new DetachOnCurrent($mi), 'cur_info_string');
103+
var_dump($mi->current());
104+
var_dump($mi->countIterators());
105+
106+
echo "-- detach inside key (assoc keys, refcounted inf) --\n";
107+
$mi = new MultipleIterator(MultipleIterator::MIT_NEED_ALL | MultipleIterator::MIT_KEYS_ASSOC);
108+
$mi->attachIterator(new DetachOnKey($mi), 'key_info_string');
109+
var_dump($mi->key());
110+
var_dump($mi->countIterators());
111+
112+
?>
113+
--EXPECT--
114+
-- detach inside rewind --
115+
rewind: still alive
116+
int(0)
117+
-- detach inside next --
118+
next: still alive
119+
int(0)
120+
-- detach inside valid --
121+
valid: still alive
122+
bool(true)
123+
int(0)
124+
-- detach inside current (numeric keys) --
125+
current: still alive
126+
array(1) {
127+
[0]=>
128+
string(1) "C"
129+
}
130+
int(0)
131+
-- detach inside key (numeric keys) --
132+
key: still alive
133+
array(1) {
134+
[0]=>
135+
string(1) "K"
136+
}
137+
int(0)
138+
-- detach inside current (assoc keys, refcounted inf) --
139+
current: still alive
140+
array(1) {
141+
["cur_info_string"]=>
142+
string(1) "C"
143+
}
144+
int(0)
145+
-- detach inside key (assoc keys, refcounted inf) --
146+
key: still alive
147+
array(1) {
148+
["key_info_string"]=>
149+
string(1) "K"
150+
}
151+
int(0)

0 commit comments

Comments
 (0)