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.

Author: ndossche

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)

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) {

ext/spl/tests/gh20101.phpt

@@ -0,0 +1,49 @@
+--TEST--
+GH-20101 (SplHeap/SplPriorityQueue serialization exposes INDIRECTs)
+--FILE--
+<?php
+class CustomHeap extends SplMaxHeap {
+    public $field = 0;
+}
+$heap = new CustomHeap();
+$data = $heap->__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) {
+    }
+  }
+}