Skip to content

Commit d29d3a4

Browse files
committed
Fix use-after-scope in SplObjectStorage::unserialize()
Introduced by the recent switch to a zend_object. Unserialize the object into a tmp_var to avoid leaving behind a stack reference. Fixes oss-fuzz #29271.
1 parent 0067c3c commit d29d3a4

File tree

2 files changed

+41
-18
lines changed

2 files changed

+41
-18
lines changed

ext/spl/spl_observer.c

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -682,7 +682,6 @@ PHP_METHOD(SplObjectStorage, unserialize)
682682
size_t buf_len;
683683
const unsigned char *p, *s;
684684
php_unserialize_data_t var_hash;
685-
zval entry, inf;
686685
zval *pcount, *pmembers;
687686
spl_SplObjectStorageElement *element;
688687
zend_long count;
@@ -715,13 +714,12 @@ PHP_METHOD(SplObjectStorage, unserialize)
715714
goto outexcept;
716715
}
717716

718-
ZVAL_UNDEF(&entry);
719-
ZVAL_UNDEF(&inf);
720-
721717
while (count-- > 0) {
722718
spl_SplObjectStorageElement *pelement;
723719
zend_hash_key key;
724-
zval obj;
720+
zval *entry = var_tmp_var(&var_hash);
721+
zval inf;
722+
ZVAL_UNDEF(&inf);
725723

726724
if (*p != ';') {
727725
goto outexcept;
@@ -731,46 +729,38 @@ PHP_METHOD(SplObjectStorage, unserialize)
731729
goto outexcept;
732730
}
733731
/* store reference to allow cross-references between different elements */
734-
if (!php_var_unserialize(&entry, &p, s + buf_len, &var_hash)) {
735-
zval_ptr_dtor(&entry);
732+
if (!php_var_unserialize(entry, &p, s + buf_len, &var_hash)) {
736733
goto outexcept;
737734
}
738735
if (*p == ',') { /* new version has inf */
739736
++p;
740737
if (!php_var_unserialize(&inf, &p, s + buf_len, &var_hash)) {
741-
zval_ptr_dtor(&entry);
742738
zval_ptr_dtor(&inf);
743739
goto outexcept;
744740
}
745741
}
746-
if (Z_TYPE(entry) != IS_OBJECT) {
747-
zval_ptr_dtor(&entry);
742+
if (Z_TYPE_P(entry) != IS_OBJECT) {
748743
zval_ptr_dtor(&inf);
749744
goto outexcept;
750745
}
751746

752-
if (spl_object_storage_get_hash(&key, intern, Z_OBJ(entry)) == FAILURE) {
753-
zval_ptr_dtor(&entry);
747+
if (spl_object_storage_get_hash(&key, intern, Z_OBJ_P(entry)) == FAILURE) {
754748
zval_ptr_dtor(&inf);
755749
goto outexcept;
756750
}
757751
pelement = spl_object_storage_get(intern, &key);
758752
spl_object_storage_free_hash(intern, &key);
759753
if (pelement) {
754+
zval obj;
760755
if (!Z_ISUNDEF(pelement->inf)) {
761756
var_push_dtor(&var_hash, &pelement->inf);
762757
}
763758
ZVAL_OBJ(&obj, pelement->obj);
764759
var_push_dtor(&var_hash, &obj);
765760
}
766-
element = spl_object_storage_attach(intern, Z_OBJ(entry), Z_ISUNDEF(inf)?NULL:&inf);
767-
ZVAL_OBJ(&obj, element->obj);
768-
var_replace(&var_hash, &entry, &obj);
761+
element = spl_object_storage_attach(intern, Z_OBJ_P(entry), Z_ISUNDEF(inf)?NULL:&inf);
769762
var_replace(&var_hash, &inf, &element->inf);
770-
zval_ptr_dtor(&entry);
771-
ZVAL_UNDEF(&entry);
772763
zval_ptr_dtor(&inf);
773-
ZVAL_UNDEF(&inf);
774764
}
775765

776766
if (*p != ';') {
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
--TEST--
2+
Reference to SplObjectStorage key (not supported)
3+
--FILE--
4+
<?php
5+
6+
$inner = 'x:i:1;O:8:"stdClass":0:{};m:a:0:{}';
7+
$inner_len = strlen($inner);
8+
$str = <<<STR
9+
a:2:{i:0;C:16:"SPlObjectStorage":{$inner_len}:{{$inner}}i:1;R:4;}
10+
STR;
11+
var_dump(unserialize($str));
12+
13+
?>
14+
--EXPECTF--
15+
array(2) {
16+
[0]=>
17+
object(SplObjectStorage)#1 (1) {
18+
["storage":"SplObjectStorage":private]=>
19+
array(1) {
20+
["%s"]=>
21+
array(2) {
22+
["obj"]=>
23+
object(stdClass)#2 (0) {
24+
}
25+
["inf"]=>
26+
NULL
27+
}
28+
}
29+
}
30+
[1]=>
31+
object(stdClass)#2 (0) {
32+
}
33+
}

0 commit comments

Comments
 (0)