Skip to content

ext/standard: Validate objects before queuing unserialize callbacks #24433

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
iliaal wants to merge 1 commit into
base: PHP-8.4
Choose a base branch
from
Open

ext/standard: Validate objects before queuing unserialize callbacks #24433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ PHP NEWS
file fails after it was already truncated. (Ilia Alshanetsky)

- Standard:
. Fixed __unserialize() and __wakeup() being called for objects with a missing
closing brace. (Ilia Alshanetsky)
. Fixed chown() and lchown() failing to resolve user names in ZTS builds
when getpwnam_r() needs a larger buffer. (Ilia Alshanetsky)
. Fixed password_get_info() and password_needs_rehash() accepting malformed
Expand Down
3 changes: 0 additions & 3 deletions ext/date/tests/unserialize-test.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ bool(false)

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


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

ossfuzz-56931.txt:
%s: unserialize(): Error at offset 39 of 39 bytes in %sunserialize-test.php on line %d

Deprecated: Creation of dynamic property DateInterval::$ is deprecated in %s on line %d
bool(false)
38 changes: 38 additions & 0 deletions ext/standard/tests/serialize/__unserialize_malformed.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
--TEST--
__unserialize() and destructors are not called for incomplete objects
--FILE--
<?php
class Test {
public function __unserialize(array $data): void {
echo "__unserialize: ";
var_dump($data);
}

public function __destruct() {
echo "__destruct\n";
}
}

foreach ([
'empty' => 'O:4:"Test":0:{',
'nonempty' => 'O:4:"Test":1:{i:0;i:42;',
'nested' => 'a:1:{i:0;O:4:"Test":0:{',
] as $name => $data) {
echo "$name: ";
var_dump(@unserialize($data));
}

echo "valid: ";
var_dump(unserialize('O:4:"Test":1:{i:0;i:42;}'));
?>
--EXPECT--
empty: bool(false)
nonempty: bool(false)
nested: bool(false)
valid: __unserialize: array(1) {
[0]=>
int(42)
}
object(Test)#1 (0) {
}
__destruct
38 changes: 38 additions & 0 deletions ext/standard/tests/serialize/__wakeup_malformed.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
--TEST--
__wakeup() and destructors are not called for incomplete objects
--FILE--
<?php
class Test {
public int $value = 0;

public function __wakeup(): void {
echo "__wakeup: $this->value\n";
}

public function __destruct() {
echo "__destruct\n";
}
}

foreach ([
'empty' => 'O:4:"Test":0:{',
'nonempty' => 'O:4:"Test":1:{s:5:"value";i:42;',
'nested' => 'a:1:{i:0;O:4:"Test":0:{',
] as $name => $data) {
echo "$name: ";
var_dump(@unserialize($data));
}

echo "valid: ";
var_dump(unserialize('O:4:"Test":1:{s:5:"value";i:42;}'));
?>
--EXPECT--
empty: bool(false)
nonempty: bool(false)
nested: bool(false)
valid: __wakeup: 42
object(Test)#1 (1) {
["value"]=>
int(42)
}
__destruct
10 changes: 6 additions & 4 deletions ext/standard/var_unserializer.re
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,8 @@ static inline int object_common(UNSERIALIZE_PARAMETER, zend_long elements, bool
array_init_size(&ary, elements);
/* Avoid reallocation due to packed -> mixed conversion. */
zend_hash_real_init_mixed(Z_ARRVAL(ary));
if (!process_nested_array_data(UNSERIALIZE_PASSTHRU, Z_ARRVAL(ary), elements)) {
if (!process_nested_array_data(UNSERIALIZE_PASSTHRU, Z_ARRVAL(ary), elements)
|| !finish_nested_data(UNSERIALIZE_PASSTHRU)) {
ZVAL_DEREF(rval);
GC_ADD_FLAGS(Z_OBJ_P(rval), IS_OBJ_DESTRUCTOR_CALLED);
zval_ptr_dtor(&ary);
Expand All @@ -815,7 +816,7 @@ static inline int object_common(UNSERIALIZE_PARAMETER, zend_long elements, bool
tmp++;
ZVAL_COPY_VALUE(tmp, &ary);

return finish_nested_data(UNSERIALIZE_PASSTHRU);
return 1;
}

has_wakeup = Z_OBJCE_P(rval) != PHP_IC_ENTRY
Expand All @@ -827,7 +828,8 @@ static inline int object_common(UNSERIALIZE_PARAMETER, zend_long elements, bool
}

zend_hash_extend(ht, zend_hash_num_elements(ht) + elements, HT_IS_PACKED(ht));
if (!process_nested_object_data(UNSERIALIZE_PASSTHRU, ht, elements, Z_OBJ_P(rval))) {
if (!process_nested_object_data(UNSERIALIZE_PASSTHRU, ht, elements, Z_OBJ_P(rval))
|| !finish_nested_data(UNSERIALIZE_PASSTHRU)) {
if (has_wakeup) {
ZVAL_DEREF(rval);
GC_ADD_FLAGS(Z_OBJ_P(rval), IS_OBJ_DESTRUCTOR_CALLED);
Expand All @@ -843,7 +845,7 @@ static inline int object_common(UNSERIALIZE_PARAMETER, zend_long elements, bool
Z_EXTRA_P(wakeup_var) = VAR_WAKEUP_FLAG;
}

return finish_nested_data(UNSERIALIZE_PASSTHRU);
return 1;
}
#ifdef PHP_WIN32
# pragma optimize("", on)
Expand Down
Loading