diff --git a/NEWS b/NEWS index de12a9a775e..998ef70e1d4 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,9 @@ PHP NEWS . Fixed bug GH-19245 (Success error message on TLS stream accept failure). (Jakub Zelenka) +- Standard: + . Fixed bug GH-16649 (UAF during array_splice). (alexandre-daubois) + 28 Aug 2025, PHP 8.3.25 - Core: diff --git a/ext/standard/array.c b/ext/standard/array.c index a2a0459ec3b..f0a81b5ba89 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -3214,6 +3214,9 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H zval *entry; /* Hash entry */ uint32_t iter_pos = zend_hash_iterators_lower_pos(in_hash, 0); + GC_ADDREF(in_hash); + HT_ALLOW_COW_VIOLATION(in_hash); /* Will be reset when setting the flags for in_hash */ + /* Get number of entries in the input hash */ num_in = zend_hash_num_elements(in_hash); @@ -3372,6 +3375,15 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H HT_SET_ITERATORS_COUNT(&out_hash, HT_ITERATORS_COUNT(in_hash)); HT_SET_ITERATORS_COUNT(in_hash, 0); in_hash->pDestructor = NULL; + + if (UNEXPECTED(GC_DELREF(in_hash) == 0)) { + /* Array was completely deallocated during the operation */ + zend_array_destroy(in_hash); + zend_hash_destroy(&out_hash); + zend_throw_error(NULL, "Array was modified during array_splice operation"); + return; + } + zend_hash_destroy(in_hash); HT_FLAGS(in_hash) = HT_FLAGS(&out_hash); diff --git a/ext/standard/tests/array/gh16649/array_splice_normal_destructor.phpt b/ext/standard/tests/array/gh16649/array_splice_normal_destructor.phpt new file mode 100644 index 00000000000..59c0b316f54 --- /dev/null +++ b/ext/standard/tests/array/gh16649/array_splice_normal_destructor.phpt @@ -0,0 +1,20 @@ +--TEST-- +GH-16649: array_splice with normal destructor should work fine +--FILE-- + +--EXPECT-- +Destructor called +array(1) { + [0]=> + string(1) "1" +} diff --git a/ext/standard/tests/array/gh16649/array_splice_uaf_add_elements.phpt b/ext/standard/tests/array/gh16649/array_splice_uaf_add_elements.phpt new file mode 100644 index 00000000000..f5e538122f4 --- /dev/null +++ b/ext/standard/tests/array/gh16649/array_splice_uaf_add_elements.phpt @@ -0,0 +1,22 @@ +--TEST-- +GH-16649: array_splice UAF when destructor adds elements to array +--FILE-- +getMessage() . "\n"; +} +?> +--EXPECT-- +Exception caught: Array was modified during array_splice operation diff --git a/ext/standard/tests/array/gh16649/array_splice_uaf_array_deallocated.phpt b/ext/standard/tests/array/gh16649/array_splice_uaf_array_deallocated.phpt new file mode 100644 index 00000000000..1874ddad893 --- /dev/null +++ b/ext/standard/tests/array/gh16649/array_splice_uaf_array_deallocated.phpt @@ -0,0 +1,22 @@ +--TEST-- +GH-16649: array_splice UAF when array is released entirely +--FILE-- +getMessage() . "\n"; +} +?> +--EXPECT-- +Exception caught: Array was modified during array_splice operation diff --git a/ext/standard/tests/array/gh16649/array_splice_uaf_complex_modification.phpt b/ext/standard/tests/array/gh16649/array_splice_uaf_complex_modification.phpt new file mode 100644 index 00000000000..8ad4133300e --- /dev/null +++ b/ext/standard/tests/array/gh16649/array_splice_uaf_complex_modification.phpt @@ -0,0 +1,25 @@ +--TEST-- +GH-16649: array_splice UAF with complex array modification +--FILE-- +getMessage() . "\n"; +} +?> +--EXPECT-- +Exception caught: Array was modified during array_splice operation diff --git a/ext/standard/tests/array/gh16649/array_splice_uaf_multiple_destructors.phpt b/ext/standard/tests/array/gh16649/array_splice_uaf_multiple_destructors.phpt new file mode 100644 index 00000000000..9e2c6cf8fc7 --- /dev/null +++ b/ext/standard/tests/array/gh16649/array_splice_uaf_multiple_destructors.phpt @@ -0,0 +1,33 @@ +--TEST-- +GH-16649: array_splice UAF with multiple destructors +--FILE-- +id = $id; + } + + function __destruct() { + global $arr; + echo "Destructor {$this->id} called\n"; + if ($this->id == 2) { + $arr = null; + } + } +} + +$arr = ["start", new MultiDestructor(1), new MultiDestructor(2), "end"]; + +try { + array_splice($arr, 1, 2); + echo "ERROR: Should have thrown exception\n"; +} catch (Error $e) { + echo "Exception caught: " . $e->getMessage() . "\n"; +} +?> +--EXPECT-- +Destructor 1 called +Destructor 2 called +Exception caught: Array was modified during array_splice operation diff --git a/ext/standard/tests/array/gh16649/array_splice_uaf_original_case.phpt b/ext/standard/tests/array/gh16649/array_splice_uaf_original_case.phpt new file mode 100644 index 00000000000..4a82d589315 --- /dev/null +++ b/ext/standard/tests/array/gh16649/array_splice_uaf_original_case.phpt @@ -0,0 +1,29 @@ +--TEST-- +GH-16649: array_splice UAF with destructor modifying array (original case) +--FILE-- + "1", "3" => new C, "2" => "2"]; + +try { + array_splice($arr, 1, 2); + echo "ERROR: Should have thrown exception\n"; +} catch (Error $e) { + echo "Exception caught: " . $e->getMessage() . "\n"; +} +?> +--EXPECT-- +Exception caught: Array was modified during array_splice operation diff --git a/ext/standard/tests/array/gh16649/array_splice_uaf_packed_to_hash.phpt b/ext/standard/tests/array/gh16649/array_splice_uaf_packed_to_hash.phpt new file mode 100644 index 00000000000..bd8f511b625 --- /dev/null +++ b/ext/standard/tests/array/gh16649/array_splice_uaf_packed_to_hash.phpt @@ -0,0 +1,23 @@ +--TEST-- +GH-16649: array_splice UAF when array is converted from packed to hash +--FILE-- +getMessage() . "\n"; +} +?> +--EXPECT-- +Exception caught: Array was modified during array_splice operation diff --git a/ext/standard/tests/array/gh16649/array_splice_with_replacement.phpt b/ext/standard/tests/array/gh16649/array_splice_with_replacement.phpt new file mode 100644 index 00000000000..8754a913a24 --- /dev/null +++ b/ext/standard/tests/array/gh16649/array_splice_with_replacement.phpt @@ -0,0 +1,23 @@ +--TEST-- +GH-16649: array_splice with replacement array when destructor modifies array +--FILE-- +getMessage() . "\n"; +} +?> +--EXPECT-- +Exception caught: Array was modified during array_splice operation