Skip to content

Commit 2821aec

Browse files
committed
Improve __unserialize() hardening for SplHeap/SplPriorityQueue
It was possible to make the heap accept unserialize data when the heap was corrupted or under modification. This adds the necessary check to prevent that from happening. Also, the exception check at the bottom is pointless, spl_heap_unserialize_internal_state() already returns FAILURE on exception. If it *is* necessary, it should be documented why.
1 parent a07d257 commit 2821aec

File tree

2 files changed

+34
-4
lines changed

2 files changed

+34
-4
lines changed

ext/spl/spl_heap.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1257,6 +1257,10 @@ PHP_METHOD(SplHeap, __unserialize)
12571257
Z_PARAM_ARRAY_HT(data)
12581258
ZEND_PARSE_PARAMETERS_END();
12591259

1260+
if (UNEXPECTED(spl_heap_consistency_validations(intern, true) != SUCCESS)) {
1261+
RETURN_THROWS();
1262+
}
1263+
12601264
if (zend_hash_num_elements(data) != 2) {
12611265
zend_throw_exception_ex(NULL, 0, "Invalid serialization data for %s object", ZSTR_VAL(intern->std.ce->name));
12621266
RETURN_THROWS();
@@ -1285,10 +1289,6 @@ PHP_METHOD(SplHeap, __unserialize)
12851289
RETURN_THROWS();
12861290
}
12871291

1288-
if (EG(exception)) {
1289-
RETURN_THROWS();
1290-
}
1291-
12921292
if (UNEXPECTED(spl_heap_consistency_validations(intern, false) != SUCCESS)) {
12931293
RETURN_THROWS();
12941294
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
--TEST--
2+
SplHeap should not accept unserialize data when it is corrupted or under modification
3+
--FILE--
4+
<?php
5+
6+
class MyHeap extends SplMaxHeap {
7+
public function compare($a, $b): int {
8+
global $array;
9+
static $counter = 0;
10+
if ($counter++ === 0)
11+
$this->__unserialize($array);
12+
return $a < $b ? -1 : ($a == $b ? 0 : 1);
13+
}
14+
}
15+
16+
$heap = new SplMaxHeap;
17+
$heap->insert(1);
18+
$array = $heap->__serialize();
19+
20+
$heap = new MyHeap;
21+
$heap->insert(0);
22+
try {
23+
$heap->insert(2);
24+
} catch (RuntimeException $e) {
25+
echo $e->getMessage(), "\n";
26+
}
27+
28+
?>
29+
--EXPECT--
30+
Heap cannot be changed when it is already being modified.

0 commit comments

Comments
 (0)