Skip to content

Commit 7cb5bdf

Browse files
pmmagakrakjoe
pmmaga
authored and
krakjoe
committed
Fix #49649 - Handle property visibility changes on unserialization
1 parent cfacf84 commit 7cb5bdf

File tree

5 files changed

+188
-7
lines changed

5 files changed

+188
-7
lines changed

‎NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ PHP NEWS
1010
. Fixed bug #74819 (wddx_deserialize() heap out-of-bound read via
1111
php_parse_date()). (Derick)
1212
. Fixed bug #74878 (Data race in ZTS builds). (Nikita)
13+
. Fixed bug #49649 (unserialize() doesn't handle changes in property
14+
visibility). (pmmaga)
1315

1416
- Date:
1517
. Fixed bug #74852 (property_exists returns true on unknown DateInterval

‎ext/standard/tests/serialize/bug49649.phpt

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
--TEST--
2+
Bug #49649 (unserialize() doesn't handle changes in property visibility) - to public
3+
--FILE--
4+
<?php
5+
6+
/**
7+
*class Foo
8+
*{
9+
* private $private = 1;
10+
*
11+
* protected $protected = 2;
12+
*
13+
* public $public = 3;
14+
*
15+
* public $notThere = 'old';
16+
* }
17+
*
18+
* echo base64_encode(serialize(new Foo()));
19+
*
20+
* The class above represents the serialized, base64_encoded string below.
21+
*/
22+
$serialized = 'TzozOiJGb28iOjQ6e3M6MTI6IgBGb28AcHJpdmF0ZSI7aToxO3M6MTI6IgAqAHByb3RlY3RlZCI7aToyO3M6NjoicHVibGljIjtpOjM7czo4OiJub3RUaGVyZSI7czozOiJvbGQiO30';
23+
24+
class Foo
25+
{
26+
public $public = null;
27+
28+
public $protected = null;
29+
30+
public $private = null;
31+
}
32+
33+
$class = unserialize(base64_decode($serialized));
34+
var_dump($class);
35+
--EXPECT--
36+
object(Foo)#1 (4) {
37+
["public"]=>
38+
int(3)
39+
["protected"]=>
40+
int(2)
41+
["private"]=>
42+
int(1)
43+
["notThere"]=>
44+
string(3) "old"
45+
}

‎ext/standard/tests/serialize/bug49649_1.phpt

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
--TEST--
2+
Bug #49649 (unserialize() doesn't handle changes in property visibility) - to protected
3+
--FILE--
4+
<?php
5+
6+
/**
7+
*class Foo
8+
*{
9+
* private $private = 1;
10+
*
11+
* protected $protected = 2;
12+
*
13+
* public $public = 3;
14+
*
15+
* public $notThere = 'old';
16+
* }
17+
*
18+
* echo base64_encode(serialize(new Foo()));
19+
*
20+
* The class above represents the serialized, base64_encoded string below.
21+
*/
22+
$serialized = 'TzozOiJGb28iOjQ6e3M6MTI6IgBGb28AcHJpdmF0ZSI7aToxO3M6MTI6IgAqAHByb3RlY3RlZCI7aToyO3M6NjoicHVibGljIjtpOjM7czo4OiJub3RUaGVyZSI7czozOiJvbGQiO30';
23+
24+
class Foo
25+
{
26+
protected $public = null;
27+
28+
protected $protected = null;
29+
30+
protected $private = null;
31+
}
32+
33+
$class = unserialize(base64_decode($serialized));
34+
var_dump($class);
35+
--EXPECT--
36+
object(Foo)#1 (4) {
37+
["public":protected]=>
38+
int(3)
39+
["protected":protected]=>
40+
int(2)
41+
["private":protected]=>
42+
int(1)
43+
["notThere"]=>
44+
string(3) "old"
45+
}

‎ext/standard/tests/serialize/bug49649_2.phpt

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
--TEST--
2+
Bug #49649 (unserialize() doesn't handle changes in property visibility) - to private
3+
--FILE--
4+
<?php
5+
6+
/**
7+
*class Foo
8+
*{
9+
* private $private = 1;
10+
*
11+
* protected $protected = 2;
12+
*
13+
* public $public = 3;
14+
*
15+
* public $notThere = 'old';
16+
* }
17+
*
18+
* echo base64_encode(serialize(new Foo()));
19+
*
20+
* The class above represents the serialized, base64_encoded string below.
21+
*/
22+
$serialized = 'TzozOiJGb28iOjQ6e3M6MTI6IgBGb28AcHJpdmF0ZSI7aToxO3M6MTI6IgAqAHByb3RlY3RlZCI7aToyO3M6NjoicHVibGljIjtpOjM7czo4OiJub3RUaGVyZSI7czozOiJvbGQiO30';
23+
24+
class Foo
25+
{
26+
private $public = null;
27+
28+
private $protected = null;
29+
30+
private $private = null;
31+
}
32+
33+
$class = unserialize(base64_decode($serialized));
34+
var_dump($class);
35+
--EXPECT--
36+
object(Foo)#1 (4) {
37+
["public":"Foo":private]=>
38+
int(3)
39+
["protected":"Foo":private]=>
40+
int(2)
41+
["private":"Foo":private]=>
42+
int(1)
43+
["notThere"]=>
44+
string(3) "old"
45+
}

‎ext/standard/var_unserializer.re

Lines changed: 51 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -427,14 +427,58 @@ numeric_key:
427427
} else {
428428
if (EXPECTED(Z_TYPE(key) == IS_STRING)) {
429429
string_key:
430-
if ((old_data = zend_hash_find(ht, Z_STR(key))) != NULL) {
431-
if (Z_TYPE_P(old_data) == IS_INDIRECT) {
432-
old_data = Z_INDIRECT_P(old_data);
430+
{
431+
zend_property_info *existing_propinfo;
432+
zend_string *new_key, *unmangled;
433+
const char *unmangled_class = NULL;
434+
const char *unmangled_prop;
435+
size_t unmangled_prop_len;
436+
437+
if (UNEXPECTED(zend_unmangle_property_name_ex(Z_STR(key), &unmangled_class, &unmangled_prop, &unmangled_prop_len) == FAILURE)) {
438+
zval_dtor(&key);
439+
return 0;
440+
}
441+
442+
unmangled = zend_string_init(unmangled_prop, unmangled_prop_len, 0);
443+
if (Z_TYPE_P(rval) == IS_OBJECT
444+
&& ((existing_propinfo = zend_hash_find_ptr(&Z_OBJCE_P(rval)->properties_info, unmangled)) != NULL)
445+
&& (existing_propinfo->flags & ZEND_ACC_PPP_MASK)) {
446+
if (existing_propinfo->flags & ZEND_ACC_PROTECTED) {
447+
new_key = zend_mangle_property_name(
448+
"*", 1, ZSTR_VAL(unmangled), ZSTR_LEN(unmangled), Z_OBJCE_P(rval)->type & ZEND_INTERNAL_CLASS);
449+
zend_string_release(unmangled);
450+
} else if (existing_propinfo->flags & ZEND_ACC_PRIVATE) {
451+
if (unmangled_class != NULL && strcmp(unmangled_class, "*") != 0) {
452+
new_key = zend_mangle_property_name(
453+
unmangled_class, strlen(unmangled_class),
454+
ZSTR_VAL(unmangled), ZSTR_LEN(unmangled),
455+
Z_OBJCE_P(rval)->type & ZEND_INTERNAL_CLASS);
456+
} else {
457+
new_key = zend_mangle_property_name(
458+
ZSTR_VAL(existing_propinfo->ce->name), ZSTR_LEN(existing_propinfo->ce->name),
459+
ZSTR_VAL(unmangled), ZSTR_LEN(unmangled),
460+
Z_OBJCE_P(rval)->type & ZEND_INTERNAL_CLASS);
461+
}
462+
zend_string_release(unmangled);
463+
} else {
464+
ZEND_ASSERT(existing_propinfo->flags & ZEND_ACC_PUBLIC);
465+
new_key = unmangled;
466+
}
467+
zend_string_release(Z_STR(key));
468+
Z_STR(key) = new_key;
469+
} else {
470+
zend_string_release(unmangled);
471+
}
472+
473+
if ((old_data = zend_hash_find(ht, Z_STR(key))) != NULL) {
474+
if (Z_TYPE_P(old_data) == IS_INDIRECT) {
475+
old_data = Z_INDIRECT_P(old_data);
476+
}
477+
var_push_dtor(var_hash, old_data);
478+
data = zend_hash_update_ind(ht, Z_STR(key), &d);
479+
} else {
480+
data = zend_hash_add_new(ht, Z_STR(key), &d);
433481
}
434-
var_push_dtor(var_hash, old_data);
435-
data = zend_hash_update_ind(ht, Z_STR(key), &d);
436-
} else {
437-
data = zend_hash_add_new(ht, Z_STR(key), &d);
438482
}
439483
} else if (Z_TYPE(key) == IS_LONG) {
440484
/* object properties should include no integers */

0 commit comments

Comments
 (0)