Skip to content

Commit 87d364d

Browse files
committed
ext/standard: Validate objects before queuing unserialize callbacks
Check the closing brace before queuing __unserialize() or __wakeup(). Incomplete object records must use the same cleanup as invalid property data, including destructor suppression. The __unserialize() data array of a failed record now stays in the unserialize context until var_destroy() instead of being released at once, so a destructor of a nested object cannot resolve a back-reference to an element the array already released.
1 parent 8934441 commit 87d364d

6 files changed

Lines changed: 112 additions & 8 deletions

File tree

‎NEWS‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,8 @@ PHP NEWS
160160
(Lazizbek Ergashev)
161161

162162
- Standard:
163+
. Fixed __unserialize() and __wakeup() being called for objects with a missing
164+
closing brace. (Ilia Alshanetsky)
163165
. Fixed bug GH-24308 (Request shutdown calls closelog() even when syslog
164166
was not opened by PHP). (lazerg)
165167
. Fixed bug GH-24399 (usleep() and time_nanosleep() ignore queued APCs on

‎ext/date/tests/unserialize-test.phpt‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ bool(false)
3131

3232
ossfuzz-55599.txt:
3333
%s: unserialize(): Error at offset 26 of 26 bytes in %sunserialize-test.php on line %d
34-
Error: Invalid serialization data for DateTime object
3534
bool(false)
3635

3736

@@ -42,6 +41,4 @@ bool(false)
4241

4342
ossfuzz-56931.txt:
4443
%s: unserialize(): Error at offset 39 of 39 bytes in %sunserialize-test.php on line %d
45-
46-
Deprecated: Creation of dynamic property DateInterval::$ is deprecated in %s on line %d
4744
bool(false)
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
__unserialize() and destructors are not called for incomplete objects
3+
--FILE--
4+
<?php
5+
class Test {
6+
public function __unserialize(array $data): void {
7+
echo "__unserialize: ";
8+
var_dump($data);
9+
}
10+
11+
public function __destruct() {
12+
echo "__destruct\n";
13+
}
14+
}
15+
16+
foreach ([
17+
'empty' => 'O:4:"Test":0:{',
18+
'nonempty' => 'O:4:"Test":1:{i:0;i:42;',
19+
'nested' => 'a:1:{i:0;O:4:"Test":0:{',
20+
] as $name => $data) {
21+
echo "$name: ";
22+
var_dump(@unserialize($data));
23+
}
24+
25+
echo "valid: ";
26+
var_dump(unserialize('O:4:"Test":1:{i:0;i:42;}'));
27+
?>
28+
--EXPECT--
29+
empty: bool(false)
30+
nonempty: bool(false)
31+
nested: bool(false)
32+
valid: __unserialize: array(1) {
33+
[0]=>
34+
int(42)
35+
}
36+
object(Test)#1 (0) {
37+
}
38+
__destruct
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
Destructor of a nested object in a failed __unserialize() payload cannot reach freed slots
3+
--FILE--
4+
<?php
5+
class T {
6+
public function __unserialize(array $data): void {}
7+
}
8+
9+
class D {
10+
public function __destruct() {
11+
var_dump(unserialize('R:2;'));
12+
}
13+
}
14+
15+
foreach ([
16+
'missing brace' => 'O:1:"T":2:{i:0;s:5:"hello";i:1;O:1:"D":0:{}',
17+
'malformed element' => 'O:1:"T":3:{i:0;s:5:"hello";i:1;O:1:"D":0:{}i:2;X',
18+
] as $name => $data) {
19+
echo "$name: ";
20+
var_dump(@unserialize($data));
21+
}
22+
?>
23+
--EXPECT--
24+
missing brace: bool(false)
25+
bool(false)
26+
malformed element: bool(false)
27+
bool(false)
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
__wakeup() and destructors are not called for incomplete objects
3+
--FILE--
4+
<?php
5+
class Test {
6+
public int $value = 0;
7+
8+
public function __wakeup(): void {
9+
echo "__wakeup: $this->value\n";
10+
}
11+
12+
public function __destruct() {
13+
echo "__destruct\n";
14+
}
15+
}
16+
17+
foreach ([
18+
'empty' => 'O:4:"Test":0:{',
19+
'nonempty' => 'O:4:"Test":1:{s:5:"value";i:42;',
20+
'nested' => 'a:1:{i:0;O:4:"Test":0:{',
21+
] as $name => $data) {
22+
echo "$name: ";
23+
var_dump(@unserialize($data));
24+
}
25+
26+
echo "valid: ";
27+
var_dump(unserialize('O:4:"Test":1:{s:5:"value";i:42;}'));
28+
?>
29+
--EXPECT--
30+
empty: bool(false)
31+
nonempty: bool(false)
32+
nested: bool(false)
33+
valid: __wakeup: 42
34+
object(Test)#1 (1) {
35+
["value"]=>
36+
int(42)
37+
}
38+
__destruct

‎ext/standard/var_unserializer.re‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -855,10 +855,11 @@ static inline int object_common(UNSERIALIZE_PARAMETER, zend_long elements, bool
855855
if (UNEXPECTED(!array_finit_size_mixed(&ary, elements))) {
856856
return 0;
857857
}
858-
if (!process_nested_array_data(UNSERIALIZE_PASSTHRU, Z_ARRVAL(ary), elements)) {
858+
if (!process_nested_array_data(UNSERIALIZE_PASSTHRU, Z_ARRVAL(ary), elements)
859+
|| !finish_nested_data(UNSERIALIZE_PASSTHRU)) {
859860
ZVAL_DEREF(rval);
860861
GC_ADD_FLAGS(Z_OBJ_P(rval), IS_OBJ_DESTRUCTOR_CALLED);
861-
zval_ptr_dtor(&ary);
862+
ZVAL_COPY_VALUE(var_tmp_var(var_hash), &ary);
862863
return 0;
863864
}
864865

@@ -871,7 +872,7 @@ static inline int object_common(UNSERIALIZE_PARAMETER, zend_long elements, bool
871872
tmp++;
872873
ZVAL_COPY_VALUE(tmp, &ary);
873874

874-
return finish_nested_data(UNSERIALIZE_PASSTHRU);
875+
return 1;
875876
}
876877

877878
has_wakeup = Z_OBJCE_P(rval) != PHP_IC_ENTRY
@@ -885,7 +886,8 @@ static inline int object_common(UNSERIALIZE_PARAMETER, zend_long elements, bool
885886
if (UNEXPECTED(zend_hash_fextend(ht, zend_hash_num_elements(ht) + elements, HT_IS_PACKED(ht)) == FAILURE)) {
886887
return 0;
887888
}
888-
if (!process_nested_object_data(UNSERIALIZE_PASSTHRU, ht, elements, Z_OBJ_P(rval))) {
889+
if (!process_nested_object_data(UNSERIALIZE_PASSTHRU, ht, elements, Z_OBJ_P(rval))
890+
|| !finish_nested_data(UNSERIALIZE_PASSTHRU)) {
889891
if (has_wakeup) {
890892
ZVAL_DEREF(rval);
891893
GC_ADD_FLAGS(Z_OBJ_P(rval), IS_OBJ_DESTRUCTOR_CALLED);
@@ -901,7 +903,7 @@ static inline int object_common(UNSERIALIZE_PARAMETER, zend_long elements, bool
901903
Z_EXTRA_P(wakeup_var) = VAR_WAKEUP_FLAG;
902904
}
903905

904-
return finish_nested_data(UNSERIALIZE_PASSTHRU);
906+
return 1;
905907
}
906908
#ifdef PHP_WIN32
907909
# pragma optimize("", on)

0 commit comments

Comments
 (0)