Skip to content

Commit 8910ac8

Browse files
committed
Fix use-after-free in ArrayObject::unset() with destructor
Fixes GH-16646 Closes GH-16653
1 parent 845cdbc commit 8910ac8

File tree

2 files changed

+35
-1
lines changed

2 files changed

+35
-1
lines changed

ext/spl/spl_array.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -555,13 +555,15 @@ static void spl_array_unset_dimension_ex(int check_inherited, zend_object *objec
555555
if (Z_TYPE_P(data) == IS_INDIRECT) {
556556
data = Z_INDIRECT_P(data);
557557
if (Z_TYPE_P(data) != IS_UNDEF) {
558-
zval_ptr_dtor(data);
558+
zval garbage;
559+
ZVAL_COPY_VALUE(&garbage, data);
559560
ZVAL_UNDEF(data);
560561
HT_FLAGS(ht) |= HASH_FLAG_HAS_EMPTY_IND;
561562
zend_hash_move_forward_ex(ht, spl_array_get_pos_ptr(ht, intern));
562563
if (spl_array_is_object(intern)) {
563564
spl_array_skip_protected(intern, ht);
564565
}
566+
zval_ptr_dtor(&garbage);
565567
}
566568
} else {
567569
zend_hash_del(ht, key.key);

ext/spl/tests/gh16646.phpt

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
--TEST--
2+
GH-16646: Use-after-free in ArrayObject::unset() with destructor
3+
--FILE--
4+
<?php
5+
6+
class B {
7+
public $b;
8+
function __construct($arg) {
9+
$this->b = $arg;
10+
}
11+
}
12+
13+
class C {
14+
function __destruct() {
15+
global $arr;
16+
echo __METHOD__, "\n";
17+
$arr->exchangeArray([]);
18+
}
19+
}
20+
21+
$arr = new ArrayObject(new B(new C));
22+
unset($arr["b"]);
23+
var_dump($arr);
24+
25+
?>
26+
--EXPECT--
27+
C::__destruct
28+
object(ArrayObject)#1 (1) {
29+
["storage":"ArrayObject":private]=>
30+
array(0) {
31+
}
32+
}

0 commit comments

Comments
 (0)