Re: com php-src: Fix #49649 - Handle property visibility changes on unserialization: NEWS ext/standard/tests/serialize/bug49649.phpt
ext/standard/tests/serialize/bug49649_1.phpt ext/standard/tests/serialize/bug49649_2.phpt ext/standard/var_unser

ACK, someone will look at it today ... Cheers Joe On Mon, Jul 10, 2017 at 10:34 AM, Dmitry Stogov <dmitry@zend.com> wrote: > This caused memory leak in Zend/tests/bug30791.phpt > > > ==13493== 20 bytes in 1 blocks are definitely lost in loss record 64 of 157 > ==13493== at 0x402C289: malloc (vg_replace_malloc.c:299) > ==13493== by 0x864C1C8: __zend_malloc (zend_alloc.c:2811) > ==13493== by 0x864B796: _emalloc (zend_alloc.c:2413) > ==13493== by 0x8550CE1: zend_string_alloc (zend_string.h:134) > ==13493== by 0x8550DDD: zend_string_init (zend_string.h:170) > ==13493== by 0x8551CE1: process_nested_data (var_unserializer.re:442) > ==13493== by 0x8552371: object_common2 (var_unserializer.re:600) > ==13493== by 0x855300F: php_var_unserialize_internal ( > var_unserializer.re:1045) > ==13493== by 0x85524B1: php_var_unserialize (var_unserializer.re:628) > ==13493== by 0x85435BC: zif_unserialize (var.c:1100) > ==13493== by 0x86C5BC9: ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER > (zend_vm_execute.h:617) > ==13493== by 0x8732538: execute_ex (zend_vm_execute.h:59096) > > > Thanks. Dmitry. > ------------------------------ > *From:* Joe Watkins <krakjoe@php.net> > *Sent:* Monday, July 10, 2017 9:27:09 AM > *To:* php-cvs@lists.php.net > *Subject:* [PHP-CVS] com php-src: Fix #49649 - Handle property visibility > changes on unserialization: NEWS ext/standard/tests/serialize/bug49649.phpt > ext/standard/tests/serialize/bug49649_1.phpt ext/standard/tests/serialize/bug49649_2.phpt > ext/standard/var_unseri... > > Commit: 7cb5bdf64a95bd70623d33d6ea122c13b01113bd > Author: Pedro Magalhães <mail@pmmaga.net> Fri, 21 Apr 2017 > 19:37:27 +0200 > Committer: Joe Watkins <krakjoe@php.net> Mon, 10 Jul 2017 07:27:09 > +0100 > Parents: cfacf84dc19c9ef497a67d09c02e27408830baa8 > Branches: master > > Link: http://git.php.net/?p=php-src.git;a=commitdiff;h= > 7cb5bdf64a95bd70623d33d6ea122c13b01113bd > > Log: > Fix #49649 - Handle property visibility changes on unserialization > > Bugs: > https://bugs.php.net/49649 > > Changed paths: > M NEWS > A ext/standard/tests/serialize/bug49649.phpt > A ext/standard/tests/serialize/bug49649_1.phpt > A ext/standard/tests/serialize/bug49649_2.phpt > M ext/standard/var_unserializer.re > > > Diff: > diff --git a/NEWS b/NEWS > index d9c1e5b..c88a6c5 100644 > --- a/NEWS > +++ b/NEWS > @@ -10,6 +10,8 @@ PHP > NEWS > . Fixed bug #74819 (wddx_deserialize() heap out-of-bound read via > php_parse_date()). (Derick) > . Fixed bug #74878 (Data race in ZTS builds). (Nikita) > + . Fixed bug #49649 (unserialize() doesn't handle changes in property > + visibility). (pmmaga) > > - Date: > . Fixed bug #74852 (property_exists returns true on unknown > DateInterval > diff --git a/ext/standard/tests/serialize/bug49649.phpt > b/ext/standard/tests/serialize/bug49649.phpt > new file mode 100644 > index 0000000..2e0a505 > --- /dev/null > +++ b/ext/standard/tests/serialize/bug49649.phpt > @@ -0,0 +1,45 @@ > +--TEST-- > +Bug #49649 (unserialize() doesn't handle changes in property visibility) > - to public > +--FILE-- > +<?php > + > +/** > + *class Foo > + *{ > + * private $private = 1; > + * > + * protected $protected = 2; > + * > + * public $public = 3; > + * > + * public $notThere = 'old'; > + * } > + * > + * echo base64_encode(serialize(new Foo())); > + * > + * The class above represents the serialized, base64_encoded string below. > +*/ > +$serialized = 'TzozOiJGb28iOjQ6e3M6MTI6IgBGb2 > 8AcHJpdmF0ZSI7aToxO3M6MTI6IgAqAHByb3RlY3RlZCI7aToyO3M6NjoicH > VibGljIjtpOjM7czo4OiJub3RUaGVyZSI7czozOiJvbGQiO30'; > + > +class Foo > +{ > + public $public = null; > + > + public $protected = null; > + > + public $private = null; > +} > + > +$class = unserialize(base64_decode($serialized)); > +var_dump($class); > +--EXPECT-- > +object(Foo)#1 (4) { > + ["public"]=> > + int(3) > + ["protected"]=> > + int(2) > + ["private"]=> > + int(1) > + ["notThere"]=> > + string(3) "old" > +} > diff --git a/ext/standard/tests/serialize/bug49649_1.phpt > b/ext/standard/tests/serialize/bug49649_1.phpt > new file mode 100644 > index 0000000..1ca840a > --- /dev/null > +++ b/ext/standard/tests/serialize/bug49649_1.phpt > @@ -0,0 +1,45 @@ > +--TEST-- > +Bug #49649 (unserialize() doesn't handle changes in property visibility) > - to protected > +--FILE-- > +<?php > + > +/** > + *class Foo > + *{ > + * private $private = 1; > + * > + * protected $protected = 2; > + * > + * public $public = 3; > + * > + * public $notThere = 'old'; > + * } > + * > + * echo base64_encode(serialize(new Foo())); > + * > + * The class above represents the serialized, base64_encoded string below. > +*/ > +$serialized = 'TzozOiJGb28iOjQ6e3M6MTI6IgBGb2 > 8AcHJpdmF0ZSI7aToxO3M6MTI6IgAqAHByb3RlY3RlZCI7aToyO3M6NjoicH > VibGljIjtpOjM7czo4OiJub3RUaGVyZSI7czozOiJvbGQiO30'; > + > +class Foo > +{ > + protected $public = null; > + > + protected $protected = null; > + > + protected $private = null; > +} > + > +$class = unserialize(base64_decode($serialized)); > +var_dump($class); > +--EXPECT-- > +object(Foo)#1 (4) { > + ["public":protected]=> > + int(3) > + ["protected":protected]=> > + int(2) > + ["private":protected]=> > + int(1) > + ["notThere"]=> > + string(3) "old" > +} > diff --git a/ext/standard/tests/serialize/bug49649_2.phpt > b/ext/standard/tests/serialize/bug49649_2.phpt > new file mode 100644 > index 0000000..e9fe429 > --- /dev/null > +++ b/ext/standard/tests/serialize/bug49649_2.phpt > @@ -0,0 +1,45 @@ > +--TEST-- > +Bug #49649 (unserialize() doesn't handle changes in property visibility) > - to private > +--FILE-- > +<?php > + > +/** > + *class Foo > + *{ > + * private $private = 1; > + * > + * protected $protected = 2; > + * > + * public $public = 3; > + * > + * public $notThere = 'old'; > + * } > + * > + * echo base64_encode(serialize(new Foo())); > + * > + * The class above represents the serialized, base64_encoded string below. > +*/ > +$serialized = 'TzozOiJGb28iOjQ6e3M6MTI6IgBGb2 > 8AcHJpdmF0ZSI7aToxO3M6MTI6IgAqAHByb3RlY3RlZCI7aToyO3M6NjoicH > VibGljIjtpOjM7czo4OiJub3RUaGVyZSI7czozOiJvbGQiO30'; > + > +class Foo > +{ > + private $public = null; > + > + private $protected = null; > + > + private $private = null; > +} > + > +$class = unserialize(base64_decode($serialized)); > +var_dump($class); > +--EXPECT-- > +object(Foo)#1 (4) { > + ["public":"Foo":private]=> > + int(3) > + ["protected":"Foo":private]=> > + int(2) > + ["private":"Foo":private]=> > + int(1) > + ["notThere"]=> > + string(3) "old" > +} > diff --git a/ext/standard/var_unserializer.re b/ext/standard/var_ > unserializer.re > index 185e62d..6e7f76f 100644 > --- a/ext/standard/var_unserializer.re > +++ b/ext/standard/var_unserializer.re > @@ -427,14 +427,58 @@ numeric_key: > } else { > if (EXPECTED(Z_TYPE(key) == IS_STRING)) { > string_key: > - if ((old_data = zend_hash_find(ht, > Z_STR(key))) != NULL) { > - if (Z_TYPE_P(old_data) == > IS_INDIRECT) { > - old_data = > Z_INDIRECT_P(old_data); > + { > + zend_property_info > *existing_propinfo; > + zend_string *new_key, *unmangled; > + const char *unmangled_class = > NULL; > + const char *unmangled_prop; > + size_t unmangled_prop_len; > + > + if > (UNEXPECTED(zend_unmangle_property_name_ex(Z_STR(key), > &unmangled_class, &unmangled_prop, &unmangled_prop_len) == FAILURE)) { > + zval_dtor(&key); > + return 0; > + } > + > + unmangled = > zend_string_init(unmangled_prop, unmangled_prop_len, 0); > + if (Z_TYPE_P(rval) == IS_OBJECT > + && > ((existing_propinfo = zend_hash_find_ptr(&Z_OBJCE_P(rval)->properties_info, > unmangled)) != NULL) > + && > (existing_propinfo->flags & ZEND_ACC_PPP_MASK)) { > + if > (existing_propinfo->flags & ZEND_ACC_PROTECTED) { > + new_key = > zend_mangle_property_name( > + "*", 1, > ZSTR_VAL(unmangled), ZSTR_LEN(unmangled), Z_OBJCE_P(rval)->type & > ZEND_INTERNAL_CLASS); > + > zend_string_release(unmangled); > + } else if > (existing_propinfo->flags & ZEND_ACC_PRIVATE) { > + if > (unmangled_class != NULL && strcmp(unmangled_class, "*") != 0) { > + new_key = > zend_mangle_property_name( > + > unmangled_class, strlen(unmangled_class), > + > ZSTR_VAL(unmangled), ZSTR_LEN(unmangled), > + > Z_OBJCE_P(rval)->type & ZEND_INTERNAL_CLASS); > + } else { > + new_key = > zend_mangle_property_name( > + > ZSTR_VAL(existing_propinfo->ce->name), ZSTR_LEN(existing_propinfo-> > ce->name), > + > ZSTR_VAL(unmangled), ZSTR_LEN(unmangled), > + > Z_OBJCE_P(rval)->type & ZEND_INTERNAL_CLASS); > + } > + > zend_string_release(unmangled); > + } else { > + > ZEND_ASSERT(existing_propinfo->flags & ZEND_ACC_PUBLIC); > + new_key = > unmangled; > + } > + > zend_string_release(Z_STR(key)); > + Z_STR(key) = new_key; > + } else { > + > zend_string_release(unmangled); > + } > + > + if ((old_data = > zend_hash_find(ht, Z_STR(key))) != NULL) { > + if (Z_TYPE_P(old_data) == > IS_INDIRECT) { > + old_data = > Z_INDIRECT_P(old_data); > + } > + var_push_dtor(var_hash, > old_data); > + data = > zend_hash_update_ind(ht, Z_STR(key), &d); > + } else { > + data = > zend_hash_add_new(ht, Z_STR(key), &d); > } > - var_push_dtor(var_hash, old_data); > - data = zend_hash_update_ind(ht, > Z_STR(key), &d); > - } else { > - data = zend_hash_add_new(ht, > Z_STR(key), &d); > } > } else if (Z_TYPE(key) == IS_LONG) { > /* object properties should include no > integers */ > > > -- > PHP CVS Mailing List (http://www.php.net/) > To unsubscribe, visit: http://www.php.net/unsub.php > >