Skip to content

Commit adb45a6

Browse files
kocsismateTysonAndre
kocsismate
and
TysonAndre
authored
Fix GH-9186 @strict-properties can be bypassed using unserialization (#9354)
* Emit deprecation warnings when adding dynamic properties to classes during unserialization - this will become an Error in php 9.0. (Adding dynamic properties in other contexts was already a deprecation warning - the use case of unserialization was overlooked) * Throw an error when attempting to add a dynamic property to a `readonly` class when unserializing * Add new serialization methods `__serialize`/`__unserialize` for SplFixedArray to avoid creating deprecated dynamic properties that would then be added to the backing fixed-size array * Don't add named dynamic/declared properties (e.g. $obj->foo) of SplFixedArray to the backing array when unserializing * Update tests to declare properties or to expect the deprecation warning * Add news entry Co-authored-by: Tyson Andre <tysonandre775@hotmail.com>
1 parent 8d78dce commit adb45a6

36 files changed

+271
-22
lines changed

‎NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ PHP NEWS
1616
(cmb)
1717
. Fixed bug GH-9285 (Traits cannot be used in readonly classes).
1818
(kocsismate)
19+
. Fixed bug GH-9186 (@strict-properties can be bypassed using
20+
unserialization). (kocsismate)
1921

2022
- Date:
2123
. Fixed bug GH-9431 (DateTime::getLastErrors() not returning false when no

‎Zend/tests/gc_043.phpt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ STR;
88
var_dump(unserialize($s));
99
gc_collect_cycles();
1010
?>
11-
--EXPECT--
11+
--EXPECTF--
12+
Deprecated: Creation of dynamic property RegexIterator::$5 is deprecated in %s on line %d
1213
object(stdClass)#1 (2) {
1314
["5"]=>
1415
object(SplStack)#2 (2) {

‎Zend/tests/readonly_classes/readonly_class_unserialize_error.phpt

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
--TEST--
2+
Fix GH-9186 Readonly classes can have dynamic properties created by unserialize()
3+
--FILE--
4+
<?php
5+
6+
readonly class C {}
7+
8+
try {
9+
$readonly = unserialize('O:1:"C":1:{s:1:"x";b:1;}');
10+
} catch (Error $exception) {
11+
echo $exception->getMessage() . "\n";
12+
}
13+
14+
?>
15+
--EXPECT--
16+
Cannot create dynamic property C::$x

‎Zend/zend_API.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1628,13 +1628,30 @@ ZEND_API void object_properties_load(zend_object *object, HashTable *properties)
16281628
zend_hash_update(object->properties, key, &tmp);
16291629
}
16301630
} else {
1631+
if (UNEXPECTED(object->ce->ce_flags & ZEND_ACC_NO_DYNAMIC_PROPERTIES)) {
1632+
zend_throw_error(NULL, "Cannot create dynamic property %s::$%s",
1633+
ZSTR_VAL(object->ce->name), property_info != ZEND_WRONG_PROPERTY_INFO ? zend_get_unmangled_property_name(key): "");
1634+
return;
1635+
} else if (!(object->ce->ce_flags & ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES)) {
1636+
zend_error(E_DEPRECATED, "Creation of dynamic property %s::$%s is deprecated",
1637+
ZSTR_VAL(object->ce->name), property_info != ZEND_WRONG_PROPERTY_INFO ? zend_get_unmangled_property_name(key): "");
1638+
}
1639+
16311640
if (!object->properties) {
16321641
rebuild_object_properties(object);
16331642
}
16341643
prop = zend_hash_update(object->properties, key, prop);
16351644
zval_add_ref(prop);
16361645
}
16371646
} else {
1647+
if (UNEXPECTED(object->ce->ce_flags & ZEND_ACC_NO_DYNAMIC_PROPERTIES)) {
1648+
zend_throw_error(NULL, "Cannot create dynamic property %s::$" ZEND_LONG_FMT, ZSTR_VAL(object->ce->name), h);
1649+
return;
1650+
} else if (!(object->ce->ce_flags & ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES)) {
1651+
zend_error(E_DEPRECATED, "Creation of dynamic property %s::$" ZEND_LONG_FMT " is deprecated",
1652+
ZSTR_VAL(object->ce->name), h);
1653+
}
1654+
16381655
if (!object->properties) {
16391656
rebuild_object_properties(object);
16401657
}

‎ext/gmp/tests/gmp_dynamic_property.phpt

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
--TEST--
2+
GH-9186 Dynamic property unserialization should trigger a deprecated notice
3+
--EXTENSIONS--
4+
gmp
5+
--FILE--
6+
<?php
7+
8+
$g = new GMP();
9+
$g->{1} = 123;
10+
11+
$serialized = serialize($g);
12+
var_dump(unserialize($serialized));
13+
14+
?>
15+
--EXPECTF--
16+
Deprecated: Creation of dynamic property GMP::$1 is deprecated in %s on line %d
17+
18+
Deprecated: Creation of dynamic property GMP::$1 is deprecated in %s on line %d
19+
object(GMP)#%d (%d) {
20+
[1]=>
21+
int(123)
22+
["num"]=>
23+
string(1) "0"
24+
}

‎ext/gmp/tests/serialize.phpt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ object(GMP)#%d (1) {
5454

5555
Deprecated: Creation of dynamic property GMP::$foo is deprecated in %s on line %d
5656
string(56) "O:3:"GMP":2:{i:0;s:1:"d";i:1;a:1:{s:3:"foo";s:3:"bar";}}"
57+
58+
Deprecated: Creation of dynamic property GMP::$foo is deprecated in %s on line %d
5759
object(GMP)#%d (2) {
5860
["foo"]=>
5961
string(3) "bar"

‎ext/random/engine_mt19937.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,10 @@ PHP_METHOD(Random_Engine_Mt19937, __unserialize)
365365
RETURN_THROWS();
366366
}
367367
object_properties_load(&engine->std, Z_ARRVAL_P(t));
368+
if (EG(exception)) {
369+
zend_throw_exception_ex(NULL, 0, "Invalid serialization data for %s object", ZSTR_VAL(engine->std.ce->name));
370+
RETURN_THROWS();
371+
}
368372

369373
/* state */
370374
t = zend_hash_index_find(d, 1);

‎ext/random/randomizer.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ PHP_METHOD(Random_Randomizer, __construct)
9090

9191
/* {{{ Generate positive random number */
9292
PHP_METHOD(Random_Randomizer, nextInt)
93-
{
93+
{
9494
php_random_randomizer *randomizer = Z_RANDOM_RANDOMIZER_P(ZEND_THIS);
9595
uint64_t result;
9696

@@ -104,7 +104,7 @@ PHP_METHOD(Random_Randomizer, nextInt)
104104
zend_throw_exception(random_ce_Random_RandomException, "Generated value exceeds size of int", 0);
105105
RETURN_THROWS();
106106
}
107-
107+
108108
RETURN_LONG((zend_long) (result >> 1));
109109
}
110110
/* }}} */
@@ -278,6 +278,10 @@ PHP_METHOD(Random_Randomizer, __unserialize)
278278
RETURN_THROWS();
279279
}
280280
object_properties_load(&randomizer->std, Z_ARRVAL_P(members_zv));
281+
if (EG(exception)) {
282+
zend_throw_exception(NULL, "Invalid serialization data for Random\\Randomizer object", 0);
283+
RETURN_THROWS();
284+
}
281285

282286
zengine = zend_read_property(randomizer->std.ce, &randomizer->std, "engine", strlen("engine"), 1, NULL);
283287
if (Z_TYPE_P(zengine) != IS_OBJECT || !instanceof_function(Z_OBJCE_P(zengine), random_ce_Random_Engine)) {

‎ext/random/tests/03_randomizer/gh_9186_unserialize.phpt

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
--TEST--
2+
Fix GH-9186 @strict-properties can be bypassed using unserialization
3+
--FILE--
4+
<?php
5+
6+
try {
7+
unserialize('O:17:"Random\Randomizer":1:{i:0;a:2:{s:3:"foo";N;s:6:"engine";O:32:"Random\Engine\Xoshiro256StarStar":2:{i:0;a:0:{}i:1;a:4:{i:0;s:16:"7520fbc2d6f8de46";i:1;s:16:"84d2d2b9d7ba0a34";i:2;s:16:"d975f36db6490b32";i:3;s:16:"c19991ee16785b94";}}}}');
8+
} catch (Exception $error) {
9+
echo $error->getMessage() . "\n";
10+
}
11+
12+
?>
13+
--EXPECT--
14+
Invalid serialization data for Random\Randomizer object

‎ext/session/tests/003.phpt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ error_reporting(E_ALL);
1616

1717
class foo {
1818
public $bar = "ok";
19+
public $yes;
1920
function method() { $this->yes++; }
2021
}
2122

‎ext/session/tests/004.phpt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ $hnd = new handler;
5252

5353
class foo {
5454
public $bar = "ok";
55+
public $yes;
5556
function method() { $this->yes++; }
5657
}
5758

‎ext/session/tests/005.phpt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ $hnd = new handler;
5353

5454
class foo {
5555
public $bar = "ok";
56+
public $yes;
5657
function method() { $this->yes++; }
5758
}
5859

‎ext/session/tests/023.phpt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ error_reporting(E_ALL);
1616

1717
class foo {
1818
public $bar = "ok";
19+
public $yes;
1920
function method() { $this->yes++; }
2021
}
2122

‎ext/session/tests/024.phpt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ $hnd = new handler;
5353

5454
class foo {
5555
public $bar = "ok";
56+
public $yes;
5657
function method() { $this->yes++; }
5758
}
5859

‎ext/session/tests/025.phpt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ $hnd = new handler;
5454

5555
class foo {
5656
public $bar = "ok";
57+
public $yes;
5758
function method() { $this->yes++; }
5859
}
5960

‎ext/soap/tests/bug70388.phpt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ Bug #70388 (SOAP serialize_function_call() type confusion / RCE)
44
soap
55
--FILE--
66
<?php
7+
8+
#[AllowDynamicProperties]
79
class MySoapClient extends SoapClient {
810
public function __doRequest($request, $location, $action, $version, $one_way = 0): string {
911
echo $request, "\n";

‎ext/soap/tests/bugs/bug69085.phpt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ soap.wsdl_cache_enabled=0
77
--FILE--
88
<?php
99

10+
#[AllowDynamicProperties]
1011
class MySoapClient extends SoapClient {
1112
public function __doRequest($request, $location, $action, $version, $one_way = 0): string {
1213
echo $request, "\n";

‎ext/spl/spl_array.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1740,6 +1740,9 @@ PHP_METHOD(ArrayObject, __unserialize)
17401740
}
17411741

17421742
object_properties_load(&intern->std, Z_ARRVAL_P(members_zv));
1743+
if (EG(exception)) {
1744+
RETURN_THROWS();
1745+
}
17431746

17441747
if (iterator_class_zv && Z_TYPE_P(iterator_class_zv) == IS_STRING) {
17451748
zend_class_entry *ce = zend_lookup_class(Z_STR_P(iterator_class_zv));

‎ext/spl/spl_fixedarray.c

Lines changed: 81 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,13 +102,18 @@ static void spl_fixedarray_init_elems(spl_fixedarray *array, zend_long from, zen
102102
}
103103
}
104104

105+
static void spl_fixedarray_init_non_empty_struct(spl_fixedarray *array, zend_long size)
106+
{
107+
array->size = 0; /* reset size in case ecalloc() fails */
108+
array->elements = size ? safe_emalloc(size, sizeof(zval), 0) : NULL;
109+
array->size = size;
110+
array->should_rebuild_properties = true;
111+
}
112+
105113
static void spl_fixedarray_init(spl_fixedarray *array, zend_long size)
106114
{
107115
if (size > 0) {
108-
array->size = 0; /* reset size in case ecalloc() fails */
109-
array->elements = safe_emalloc(size, sizeof(zval), 0);
110-
array->size = size;
111-
array->should_rebuild_properties = true;
116+
spl_fixedarray_init_non_empty_struct(array, size);
112117
spl_fixedarray_init_elems(array, 0, size);
113118
} else {
114119
spl_fixedarray_default_ctor(array);
@@ -582,6 +587,78 @@ PHP_METHOD(SplFixedArray, __wakeup)
582587
}
583588
}
584589

590+
PHP_METHOD(SplFixedArray, __serialize)
591+
{
592+
spl_fixedarray_object *intern = Z_SPLFIXEDARRAY_P(ZEND_THIS);
593+
zval *current;
594+
zend_string *key;
595+
596+
if (zend_parse_parameters_none() == FAILURE) {
597+
RETURN_THROWS();
598+
}
599+
600+
uint32_t property_num = zend_hash_num_elements(intern->std.properties);
601+
array_init_size(return_value, intern->array.size + property_num);
602+
603+
/* elements */
604+
for (zend_long i = 0; i < intern->array.size; i++) {
605+
current = &intern->array.elements[i];
606+
zend_hash_next_index_insert(Z_ARRVAL_P(return_value), current);
607+
Z_TRY_ADDREF_P(current);
608+
}
609+
610+
/* members */
611+
ZEND_HASH_FOREACH_STR_KEY_VAL(intern->std.properties, key, current) {
612+
zend_hash_add(Z_ARRVAL_P(return_value), key, current);
613+
Z_TRY_ADDREF_P(current);
614+
} ZEND_HASH_FOREACH_END();
615+
}
616+
617+
PHP_METHOD(SplFixedArray, __unserialize)
618+
{
619+
spl_fixedarray_object *intern = Z_SPLFIXEDARRAY_P(ZEND_THIS);
620+
HashTable *data;
621+
zval members_zv, *elem;
622+
zend_string *key;
623+
zend_long size;
624+
625+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "h", &data) == FAILURE) {
626+
RETURN_THROWS();
627+
}
628+
629+
if (intern->array.size == 0) {
630+
size = zend_hash_num_elements(data);
631+
spl_fixedarray_init_non_empty_struct(&intern->array, size);
632+
if (!size) {
633+
return;
634+
}
635+
array_init(&members_zv);
636+
637+
intern->array.size = 0;
638+
ZEND_HASH_FOREACH_STR_KEY_VAL(data, key, elem) {
639+
if (key == NULL) {
640+
ZVAL_COPY(&intern->array.elements[intern->array.size], elem);
641+
intern->array.size++;
642+
} else {
643+
Z_TRY_ADDREF_P(elem);
644+
zend_hash_add(Z_ARRVAL(members_zv), key, elem);
645+
}
646+
} ZEND_HASH_FOREACH_END();
647+
648+
if (intern->array.size != size) {
649+
if (intern->array.size) {
650+
intern->array.elements = erealloc(intern->array.elements, sizeof(zval) * intern->array.size);
651+
} else {
652+
efree(intern->array.elements);
653+
intern->array.elements = NULL;
654+
}
655+
}
656+
657+
object_properties_load(&intern->std, Z_ARRVAL(members_zv));
658+
zval_ptr_dtor(&members_zv);
659+
}
660+
}
661+
585662
PHP_METHOD(SplFixedArray, count)
586663
{
587664
zval *object = ZEND_THIS;

‎ext/spl/spl_fixedarray.stub.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ public function __construct(int $size = 0) {}
99
/** @tentative-return-type */
1010
public function __wakeup(): void {}
1111

12+
public function __serialize(): array {}
13+
14+
public function __unserialize(array $data): void {}
15+
1216
/** @tentative-return-type */
1317
public function count(): int {}
1418

‎ext/spl/spl_fixedarray_arginfo.h

Lines changed: 13 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)