Skip to content

Commit 9170405

Browse files
committed
Fix array going away during sorting
Fixes GH-16648
1 parent 9e5024d commit 9170405

File tree

2 files changed

+63
-0
lines changed

2 files changed

+63
-0
lines changed

Zend/tests/gh16648.phpt

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
--TEST--
2+
GH-16648: Use-after-free during array sorting
3+
--FILE--
4+
<?php
5+
6+
function resize_arr() {
7+
global $arr;
8+
for ($i = 0; $i < 10; $i++) {
9+
$arr[$i] = $i;
10+
}
11+
}
12+
13+
class C {
14+
function __tostring() {
15+
resize_arr();
16+
return "3";
17+
}
18+
}
19+
20+
$arr = ["a" => "1", "3" => new C, "2" => "2"];
21+
asort($arr);
22+
var_dump($arr);
23+
24+
?>
25+
--EXPECT--
26+
array(11) {
27+
["a"]=>
28+
string(1) "1"
29+
[3]=>
30+
int(3)
31+
[2]=>
32+
int(2)
33+
[0]=>
34+
int(0)
35+
[1]=>
36+
int(1)
37+
[4]=>
38+
int(4)
39+
[5]=>
40+
int(5)
41+
[6]=>
42+
int(6)
43+
[7]=>
44+
int(7)
45+
[8]=>
46+
int(8)
47+
[9]=>
48+
int(9)
49+
}

Zend/zend_hash.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2880,6 +2880,10 @@ ZEND_API void ZEND_FASTCALL zend_hash_sort_ex(HashTable *ht, sort_func_t sort, b
28802880
zend_hash_packed_to_hash(ht); // TODO: ???
28812881
}
28822882

2883+
ZEND_ASSERT(!(GC_FLAGS(ht) & IS_ARRAY_IMMUTABLE));
2884+
/* Adding a refcount prevents the array from going away. */
2885+
GC_ADDREF(ht);
2886+
28832887
if (HT_IS_WITHOUT_HOLES(ht)) {
28842888
/* Store original order of elements in extra space to allow stable sorting. */
28852889
for (i = 0; i < ht->nNumUsed; i++) {
@@ -2954,6 +2958,16 @@ ZEND_API void ZEND_FASTCALL zend_hash_sort_ex(HashTable *ht, sort_func_t sort, b
29542958
zend_hash_rehash(ht);
29552959
}
29562960
}
2961+
2962+
if (UNEXPECTED(GC_DELREF(ht) == 0)) {
2963+
zend_array_destroy(ht);
2964+
} else {
2965+
/* The array _should_ be added to the gc buffer. However, this causes an
2966+
* issue for non-user hashtables that use zend_hash_destroy(), which does
2967+
* not remove the destroyed array from the gc buffer, assuming it was
2968+
* never added. */
2969+
// gc_check_possible_root((zend_refcounted *)ht);
2970+
}
29572971
}
29582972

29592973
static zend_always_inline int zend_hash_compare_impl(HashTable *ht1, HashTable *ht2, compare_func_t compar, bool ordered) {

0 commit comments

Comments
 (0)