Skip to content

Commit 0a84d23

Browse files
committed
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.
1 parent 52e1c9f commit 0a84d23

File tree

4 files changed

+56
-32
lines changed

4 files changed

+56
-32
lines changed

ext/spl/spl_heap.c

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1185,8 +1185,7 @@ PHP_METHOD(SplPriorityQueue, __serialize)
11851185

11861186
array_init(return_value);
11871187

1188-
ZVAL_ARR(&props, zend_std_get_properties(&intern->std));
1189-
Z_TRY_ADDREF(props);
1188+
ZVAL_ARR(&props, zend_array_dup(zend_std_get_properties(&intern->std)));
11901189
zend_hash_next_index_insert(Z_ARRVAL_P(return_value), &props);
11911190

11921191
spl_heap_serialize_internal_state(&state, intern, true);
@@ -1239,32 +1238,6 @@ PHP_METHOD(SplPriorityQueue, __unserialize)
12391238
}
12401239
}
12411240

1242-
PHP_METHOD(SplHeap, __serialize)
1243-
{
1244-
spl_heap_object *intern = Z_SPLHEAP_P(ZEND_THIS);
1245-
zval props, state;
1246-
1247-
ZEND_PARSE_PARAMETERS_NONE();
1248-
1249-
if (UNEXPECTED(spl_heap_consistency_validations(intern, false) != SUCCESS)) {
1250-
RETURN_THROWS();
1251-
}
1252-
1253-
if (intern->heap->flags & SPL_HEAP_WRITE_LOCKED) {
1254-
zend_throw_exception(spl_ce_RuntimeException, "Cannot serialize heap while it is being modified.", 0);
1255-
RETURN_THROWS();
1256-
}
1257-
1258-
array_init(return_value);
1259-
1260-
ZVAL_ARR(&props, zend_std_get_properties(&intern->std));
1261-
Z_TRY_ADDREF(props);
1262-
zend_hash_next_index_insert(Z_ARRVAL_P(return_value), &props);
1263-
1264-
spl_heap_serialize_internal_state(&state, intern, false);
1265-
zend_hash_next_index_insert(Z_ARRVAL_P(return_value), &state);
1266-
}
1267-
12681241
PHP_METHOD(SplHeap, __unserialize)
12691242
{
12701243
HashTable *data;

ext/spl/spl_heap.stub.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,10 @@ public function isCorrupted(): bool {}
134134
/** @tentative-return-type */
135135
public function __debugInfo(): array {}
136136

137-
/** @tentative-return-type */
137+
/**
138+
* @tentative-return-type
139+
* @implementation-alias SplPriorityQueue::__serialize
140+
*/
138141
public function __serialize(): array {}
139142

140143
/** @tentative-return-type */

ext/spl/spl_heap_arginfo.h

Lines changed: 2 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

ext/spl/tests/gh20101.phpt

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
--TEST--
2+
GH-20101 (SplHeap/SplPriorityQueue serialization exposes INDIRECTs)
3+
--FILE--
4+
<?php
5+
class CustomHeap extends SplMaxHeap {
6+
public $field = 0;
7+
}
8+
$heap = new CustomHeap();
9+
$data = $heap->__serialize();
10+
var_dump($data);
11+
12+
class CustomPriorityQueue extends SplPriorityQueue {
13+
public $field = 0;
14+
}
15+
$pqueue = new CustomPriorityQueue();
16+
$data = $pqueue->__serialize();
17+
var_dump($data);
18+
?>
19+
--EXPECT--
20+
array(2) {
21+
[0]=>
22+
array(1) {
23+
["field"]=>
24+
int(0)
25+
}
26+
[1]=>
27+
array(2) {
28+
["flags"]=>
29+
int(0)
30+
["heap_elements"]=>
31+
array(0) {
32+
}
33+
}
34+
}
35+
array(2) {
36+
[0]=>
37+
array(1) {
38+
["field"]=>
39+
int(0)
40+
}
41+
[1]=>
42+
array(2) {
43+
["flags"]=>
44+
int(1)
45+
["heap_elements"]=>
46+
array(0) {
47+
}
48+
}
49+
}

0 commit comments

Comments
 (0)