Skip to content

Commit 230defc

Browse files
committed
Merge branch 'PHP-8.3' into PHP-8.4
* PHP-8.3: Fix array going away during sorting
2 parents ea39a89 + f033cf7 commit 230defc

File tree

6 files changed

+94
-10
lines changed

6 files changed

+94
-10
lines changed

NEWS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ PHP NEWS
99
(nielsdos)
1010
. Fixed bug GH-16615 (Assertion failure in zend_std_read_property). (Arnaud)
1111
. Fixed bug GH-16342 (Added ReflectionProperty::isLazy()). (Arnaud)
12+
. Fixed bug GH-16648 (Use-after-free during array sorting). (ilutov)
1213

1314
- Date:
1415
. Fixed bug GH-14732 (date_sun_info() fails for non-finite values). (cmb)

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: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2973,13 +2973,12 @@ ZEND_API void zend_hash_bucket_packed_swap(Bucket *p, Bucket *q)
29732973
q->h = h;
29742974
}
29752975

2976-
ZEND_API void ZEND_FASTCALL zend_hash_sort_ex(HashTable *ht, sort_func_t sort, bucket_compare_func_t compar, bool renumber)
2976+
static void zend_hash_sort_internal(HashTable *ht, sort_func_t sort, bucket_compare_func_t compar, bool renumber)
29772977
{
29782978
Bucket *p;
29792979
uint32_t i, j;
29802980

29812981
IS_CONSISTENT(ht);
2982-
HT_ASSERT_RC1(ht);
29832982

29842983
if (!(ht->nNumOfElements>1) && !(renumber && ht->nNumOfElements>0)) {
29852984
/* Doesn't require sorting */
@@ -3066,6 +3065,33 @@ ZEND_API void ZEND_FASTCALL zend_hash_sort_ex(HashTable *ht, sort_func_t sort, b
30663065
}
30673066
}
30683067

3068+
ZEND_API void ZEND_FASTCALL zend_hash_sort_ex(HashTable *ht, sort_func_t sort, bucket_compare_func_t compar, bool renumber)
3069+
{
3070+
HT_ASSERT_RC1(ht);
3071+
zend_hash_sort_internal(ht, sort, compar, renumber);
3072+
}
3073+
3074+
void ZEND_FASTCALL zend_array_sort_ex(HashTable *ht, sort_func_t sort, bucket_compare_func_t compar, bool renumber)
3075+
{
3076+
HT_ASSERT_RC1(ht);
3077+
3078+
/* Unpack the array early to avoid RCn assertion failures. */
3079+
if (HT_IS_PACKED(ht)) {
3080+
zend_hash_packed_to_hash(ht);
3081+
}
3082+
3083+
/* Adding a refcount prevents the array from going away. */
3084+
GC_ADDREF(ht);
3085+
3086+
zend_hash_sort_internal(ht, sort, compar, renumber);
3087+
3088+
if (UNEXPECTED(GC_DELREF(ht) == 0)) {
3089+
zend_array_destroy(ht);
3090+
} else {
3091+
gc_check_possible_root((zend_refcounted *)ht);
3092+
}
3093+
}
3094+
30693095
static zend_always_inline int zend_hash_compare_impl(HashTable *ht1, HashTable *ht2, compare_func_t compar, bool ordered) {
30703096
uint32_t idx1, idx2;
30713097
zend_string *key1, *key2;

Zend/zend_hash.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,12 +299,20 @@ ZEND_API void zend_hash_bucket_packed_swap(Bucket *p, Bucket *q);
299299
typedef int (*bucket_compare_func_t)(Bucket *a, Bucket *b);
300300
ZEND_API int zend_hash_compare(HashTable *ht1, HashTable *ht2, compare_func_t compar, bool ordered);
301301
ZEND_API void ZEND_FASTCALL zend_hash_sort_ex(HashTable *ht, sort_func_t sort_func, bucket_compare_func_t compare_func, bool renumber);
302+
void ZEND_FASTCALL zend_array_sort_ex(HashTable *ht, sort_func_t sort_func, bucket_compare_func_t compare_func, bool renumber);
302303
ZEND_API zval* ZEND_FASTCALL zend_hash_minmax(const HashTable *ht, compare_func_t compar, uint32_t flag);
303304

304305
static zend_always_inline void ZEND_FASTCALL zend_hash_sort(HashTable *ht, bucket_compare_func_t compare_func, bool renumber) {
305306
zend_hash_sort_ex(ht, zend_sort, compare_func, renumber);
306307
}
307308

309+
/* Use this variant over zend_hash_sort() when sorting user arrays that may
310+
* trigger user code. It will ensure the user code cannot free the array during
311+
* sorting. */
312+
static zend_always_inline void zend_array_sort(HashTable *ht, bucket_compare_func_t compare_func, zend_bool renumber) {
313+
zend_array_sort_ex(ht, zend_sort, compare_func, renumber);
314+
}
315+
308316
static zend_always_inline uint32_t zend_hash_num_elements(const HashTable *ht) {
309317
return ht->nNumOfElements;
310318
}

ext/intl/collator/collator_sort.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@ static void collator_sort_internal( int renumber, INTERNAL_FUNCTION_PARAMETERS )
292292
INTL_G( current_collator ) = co->ucoll;
293293

294294
/* Sort specified array. */
295-
zend_hash_sort(hash, collator_compare_func, renumber);
295+
zend_array_sort(hash, collator_compare_func, renumber);
296296

297297
/* Restore saved collator. */
298298
INTL_G( current_collator ) = saved_collator;

ext/standard/array.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -700,9 +700,9 @@ static void php_natsort(INTERNAL_FUNCTION_PARAMETERS, int fold_case) /* {{{ */
700700
ZEND_PARSE_PARAMETERS_END();
701701

702702
if (fold_case) {
703-
zend_hash_sort(Z_ARRVAL_P(array), php_array_natural_case_compare, 0);
703+
zend_array_sort(Z_ARRVAL_P(array), php_array_natural_case_compare, 0);
704704
} else {
705-
zend_hash_sort(Z_ARRVAL_P(array), php_array_natural_compare, 0);
705+
zend_array_sort(Z_ARRVAL_P(array), php_array_natural_compare, 0);
706706
}
707707

708708
RETURN_TRUE;
@@ -738,7 +738,7 @@ PHP_FUNCTION(asort)
738738

739739
cmp = php_get_data_compare_func(sort_type, 0);
740740

741-
zend_hash_sort(Z_ARRVAL_P(array), cmp, 0);
741+
zend_array_sort(Z_ARRVAL_P(array), cmp, 0);
742742

743743
RETURN_TRUE;
744744
}
@@ -759,7 +759,7 @@ PHP_FUNCTION(arsort)
759759

760760
cmp = php_get_data_compare_func(sort_type, 1);
761761

762-
zend_hash_sort(Z_ARRVAL_P(array), cmp, 0);
762+
zend_array_sort(Z_ARRVAL_P(array), cmp, 0);
763763

764764
RETURN_TRUE;
765765
}
@@ -780,7 +780,7 @@ PHP_FUNCTION(sort)
780780

781781
cmp = php_get_data_compare_func(sort_type, 0);
782782

783-
zend_hash_sort(Z_ARRVAL_P(array), cmp, 1);
783+
zend_array_sort(Z_ARRVAL_P(array), cmp, 1);
784784

785785
RETURN_TRUE;
786786
}
@@ -801,7 +801,7 @@ PHP_FUNCTION(rsort)
801801

802802
cmp = php_get_data_compare_func(sort_type, 1);
803803

804-
zend_hash_sort(Z_ARRVAL_P(array), cmp, 1);
804+
zend_array_sort(Z_ARRVAL_P(array), cmp, 1);
805805

806806
RETURN_TRUE;
807807
}
@@ -899,7 +899,7 @@ static void php_usort(INTERNAL_FUNCTION_PARAMETERS, bucket_compare_func_t compar
899899
/* Copy array, so the in-place modifications will not be visible to the callback function */
900900
arr = zend_array_dup(arr);
901901

902-
zend_hash_sort(arr, compare_func, renumber);
902+
zend_array_sort(arr, compare_func, renumber);
903903

904904
zval garbage;
905905
ZVAL_COPY_VALUE(&garbage, array);

0 commit comments

Comments
 (0)