Skip to content

Commit 886a528

Browse files
committed
Fix phpGH-16604: Memory leaks in SPL constructors
Closes phpGH-16673.
1 parent ec68d3c commit 886a528

File tree

5 files changed

+61
-11
lines changed

5 files changed

+61
-11
lines changed

NEWS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ PHP NEWS
113113
. Fixed bug GH-16589 (UAF in SplDoublyLinked->serialize()). (nielsdos)
114114
. Fixed bug GH-14687 (segfault on SplObjectIterator instance).
115115
(David Carlier)
116+
. Fixed bug GH-16604 (Memory leaks in SPL constructors). (nielsdos)
116117

117118
- Standard:
118119
. Fixed bug GH-16293 (Failed assertion when throwing in assert() callback with

ext/spl/spl_directory.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2053,6 +2053,12 @@ PHP_METHOD(SplFileObject, __construct)
20532053
RETURN_THROWS();
20542054
}
20552055

2056+
/* Prevent reinitialization of Object */
2057+
if (UNEXPECTED(intern->u.file.stream)) {
2058+
zend_throw_error(NULL, "Cannot call constructor twice");
2059+
RETURN_THROWS();
2060+
}
2061+
20562062
intern->u.file.open_mode = zend_string_copy(open_mode);
20572063
/* file_name and zcontext are copied by spl_filesystem_file_open() */
20582064
intern->file_name = file_name;
@@ -2096,7 +2102,7 @@ PHP_METHOD(SplTempFileObject, __construct)
20962102
}
20972103

20982104
/* Prevent reinitialization of Object */
2099-
if (intern->u.file.stream) {
2105+
if (UNEXPECTED(intern->u.file.stream)) {
21002106
zend_throw_error(NULL, "Cannot call constructor twice");
21012107
RETURN_THROWS();
21022108
}

ext/spl/spl_iterators.c

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,20 @@ static int spl_get_iterator_from_aggregate(zval *retval, zend_class_entry *ce, z
538538
return SUCCESS;
539539
}
540540

541+
static void spl_RecursiveIteratorIterator_free_iterators(spl_recursive_it_object *object)
542+
{
543+
if (object->iterators) {
544+
while (object->level >= 0) {
545+
zend_object_iterator *sub_iter = object->iterators[object->level].iterator;
546+
zend_iterator_dtor(sub_iter);
547+
zval_ptr_dtor(&object->iterators[object->level].zobject);
548+
object->level--;
549+
}
550+
efree(object->iterators);
551+
object->iterators = NULL;
552+
}
553+
}
554+
541555
static void spl_recursive_it_it_construct(INTERNAL_FUNCTION_PARAMETERS, zend_class_entry *ce_base, zend_class_entry *ce_inner, recursive_it_it_type rit_type)
542556
{
543557
zval *object = ZEND_THIS;
@@ -604,6 +618,7 @@ static void spl_recursive_it_it_construct(INTERNAL_FUNCTION_PARAMETERS, zend_cla
604618
}
605619

606620
intern = Z_SPLRECURSIVE_IT_P(object);
621+
spl_RecursiveIteratorIterator_free_iterators(intern);
607622
intern->iterators = emalloc(sizeof(spl_sub_iterator));
608623
intern->level = 0;
609624
intern->mode = mode;
@@ -650,6 +665,7 @@ static void spl_recursive_it_it_construct(INTERNAL_FUNCTION_PARAMETERS, zend_cla
650665
intern->iterators[0].getchildren = NULL;
651666

652667
if (EG(exception)) {
668+
// TODO: use spl_RecursiveIteratorIterator_free_iterators
653669
zend_object_iterator *sub_iter;
654670

655671
while (intern->level >= 0) {
@@ -958,16 +974,7 @@ static void spl_RecursiveIteratorIterator_free_storage(zend_object *_object)
958974
{
959975
spl_recursive_it_object *object = spl_recursive_it_from_obj(_object);
960976

961-
if (object->iterators) {
962-
while (object->level >= 0) {
963-
zend_object_iterator *sub_iter = object->iterators[object->level].iterator;
964-
zend_iterator_dtor(sub_iter);
965-
zval_ptr_dtor(&object->iterators[object->level].zobject);
966-
object->level--;
967-
}
968-
efree(object->iterators);
969-
object->iterators = NULL;
970-
}
977+
spl_RecursiveIteratorIterator_free_iterators(object);
971978

972979
zend_object_std_dtor(&object->std);
973980
for (size_t i = 0; i < 6; i++) {

ext/spl/tests/gh16604_1.phpt

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
--TEST--
2+
GH-16604 (Memory leaks in SPL constructors) - recursive iterators
3+
--FILE--
4+
<?php
5+
6+
$traversable = new RecursiveArrayIterator( [] );
7+
8+
$obj = new RecursiveIteratorIterator( $traversable );
9+
$obj->__construct( $traversable );
10+
11+
$obj = new RecursiveTreeIterator( $traversable );
12+
$obj->__construct( $traversable );
13+
14+
?>
15+
--EXPECT--

ext/spl/tests/gh16604_2.phpt

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
--TEST--
2+
GH-16604 (Memory leaks in SPL constructors) - SplFileObject
3+
--FILE--
4+
<?php
5+
6+
file_put_contents(__DIR__.'/gh16604_2.tmp', 'hello');
7+
8+
$obj = new SplFileObject(__DIR__.'/gh16604_2.tmp');
9+
try {
10+
$obj->__construct(__DIR__.'/gh16604_2.tmp');
11+
} catch (Error $e) {
12+
echo $e->getMessage(), "\n";
13+
}
14+
15+
?>
16+
--CLEAN--
17+
<?php
18+
@unlink(__DIR__.'/gh16604_2.tmp');
19+
?>
20+
--EXPECT--
21+
Cannot call constructor twice

0 commit comments

Comments
 (0)