From 04a55167596047aed8dfedfb51d1005a358e3c0d 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. --- ext/spl/spl_heap.c | 35 ++++--------- .../SplHeap_serialize_indexed_format.phpt | 4 +- ext/spl/tests/gh20101.phpt | 49 +++++++++++++++++++ 3 files changed, 60 insertions(+), 28 deletions(-) create mode 100644 ext/spl/tests/gh20101.phpt diff --git a/ext/spl/spl_heap.c b/ext/spl/spl_heap.c index 19e86b0d80b72..254fbde7b3ff0 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 c201feb5716f6..21ea74c6bff9b 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 0000000000000..ad2399d76c8e6 --- /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) { + } + } +}