From 0458b3c8db791ebc569a5bf30111c6396a5266cc Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 8 Oct 2025 17:28:37 +0200 Subject: [PATCH] Fix GH-20101: SplHeap/SplPriorityQueue serialization exposes INDIRECTs Exposing INDIRECTs to userland is not allowed and can lead to all sorts of wrong behaviour. In this case it lead to UAF bugs. Solve it by duplicating the properties table, which de-indirects the elements and also decouples it for future modifications. Closes GH-20102. --- NEWS | 3 ++ ext/spl/spl_heap.c | 35 ++++--------- .../SplHeap_serialize_indexed_format.phpt | 4 +- ext/spl/tests/gh20101.phpt | 49 +++++++++++++++++++ 4 files changed, 63 insertions(+), 28 deletions(-) create mode 100644 ext/spl/tests/gh20101.phpt diff --git a/NEWS b/NEWS index 93b2616f71f..4b09eacae5d 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,9 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.5.0RC3 +- SPL: + . Fixed bug GH-20101 (SplHeap/SplPriorityQueue serialization + exposes INDIRECTs). (nielsdos) 09 Oct 2025, PHP 8.5.0RC2 diff --git a/ext/spl/spl_heap.c b/ext/spl/spl_heap.c index 19e86b0d80b..254fbde7b3f 100644 --- a/ext/spl/spl_heap.c +++ b/ext/spl/spl_heap.c @@ -1167,7 +1167,7 @@ static zend_result spl_heap_unserialize_internal_state(HashTable *state_ht, spl_ return SUCCESS; } -PHP_METHOD(SplPriorityQueue, __serialize) +static void spl_heap_serialize_internal(INTERNAL_FUNCTION_PARAMETERS, bool is_pqueue) { spl_heap_object *intern = Z_SPLHEAP_P(ZEND_THIS); zval props, state; @@ -1185,14 +1185,18 @@ PHP_METHOD(SplPriorityQueue, __serialize) array_init(return_value); - ZVAL_ARR(&props, zend_std_get_properties(&intern->std)); - Z_TRY_ADDREF(props); + ZVAL_ARR(&props, zend_array_dup(zend_std_get_properties(&intern->std))); zend_hash_next_index_insert(Z_ARRVAL_P(return_value), &props); - spl_heap_serialize_internal_state(&state, intern, true); + spl_heap_serialize_internal_state(&state, intern, is_pqueue); zend_hash_next_index_insert(Z_ARRVAL_P(return_value), &state); } +PHP_METHOD(SplPriorityQueue, __serialize) +{ + spl_heap_serialize_internal(INTERNAL_FUNCTION_PARAM_PASSTHRU, true); +} + PHP_METHOD(SplPriorityQueue, __unserialize) { HashTable *data; @@ -1241,28 +1245,7 @@ PHP_METHOD(SplPriorityQueue, __unserialize) PHP_METHOD(SplHeap, __serialize) { - spl_heap_object *intern = Z_SPLHEAP_P(ZEND_THIS); - zval props, state; - - ZEND_PARSE_PARAMETERS_NONE(); - - if (UNEXPECTED(spl_heap_consistency_validations(intern, false) != SUCCESS)) { - RETURN_THROWS(); - } - - if (intern->heap->flags & SPL_HEAP_WRITE_LOCKED) { - zend_throw_exception(spl_ce_RuntimeException, "Cannot serialize heap while it is being modified.", 0); - RETURN_THROWS(); - } - - array_init(return_value); - - ZVAL_ARR(&props, zend_std_get_properties(&intern->std)); - Z_TRY_ADDREF(props); - zend_hash_next_index_insert(Z_ARRVAL_P(return_value), &props); - - spl_heap_serialize_internal_state(&state, intern, false); - zend_hash_next_index_insert(Z_ARRVAL_P(return_value), &state); + spl_heap_serialize_internal(INTERNAL_FUNCTION_PARAM_PASSTHRU, false); } PHP_METHOD(SplHeap, __unserialize) diff --git a/ext/spl/tests/SplHeap_serialize_indexed_format.phpt b/ext/spl/tests/SplHeap_serialize_indexed_format.phpt index c201feb5716..21ea74c6bff 100644 --- a/ext/spl/tests/SplHeap_serialize_indexed_format.phpt +++ b/ext/spl/tests/SplHeap_serialize_indexed_format.phpt @@ -74,9 +74,9 @@ array(2) { [0]=> array(2) { ["flags"]=> - UNKNOWN:0 + string(13) "user_property" ["heap_elements"]=> - UNKNOWN:0 + string(13) "user_property" } [1]=> array(2) { diff --git a/ext/spl/tests/gh20101.phpt b/ext/spl/tests/gh20101.phpt new file mode 100644 index 00000000000..ad2399d76c8 --- /dev/null +++ b/ext/spl/tests/gh20101.phpt @@ -0,0 +1,49 @@ +--TEST-- +GH-20101 (SplHeap/SplPriorityQueue serialization exposes INDIRECTs) +--FILE-- +__serialize(); +var_dump($data); + +class CustomPriorityQueue extends SplPriorityQueue { + public $field = 0; +} +$pqueue = new CustomPriorityQueue(); +$data = $pqueue->__serialize(); +var_dump($data); +?> +--EXPECT-- +array(2) { + [0]=> + array(1) { + ["field"]=> + int(0) + } + [1]=> + array(2) { + ["flags"]=> + int(0) + ["heap_elements"]=> + array(0) { + } + } +} +array(2) { + [0]=> + array(1) { + ["field"]=> + int(0) + } + [1]=> + array(2) { + ["flags"]=> + int(1) + ["heap_elements"]=> + array(0) { + } + } +}