From b2b7519b6e6e0f8d6243b166c8c92455e412b734 Mon Sep 17 00:00:00 2001 From: Martin Hoch Date: Tue, 23 May 2023 17:39:17 +0200 Subject: [PATCH 1/2] serialize: Fixed handling of nested object references The removed if in var.c caused serialize to not handle object references correctly under certain circumstances. See tests/serialize/serialization_objects_019.phpt The bug was originally introduced in commit 6c5942f, and the problematic line was last modified in commit bb0b4eb9. (Fixes oss-fuzz #44954) The testcase from bb0b4eb9 still passes. --- .../serialize/serialization_objects_019.phpt | 17 +++++++++++++++++ ext/standard/var.c | 2 -- 2 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 ext/standard/tests/serialize/serialization_objects_019.phpt diff --git a/ext/standard/tests/serialize/serialization_objects_019.phpt b/ext/standard/tests/serialize/serialization_objects_019.phpt new file mode 100644 index 0000000000000..505fc3d9af739 --- /dev/null +++ b/ext/standard/tests/serialize/serialization_objects_019.phpt @@ -0,0 +1,17 @@ +--TEST-- +Object serialization with references +--FILE-- +a = [$s]; + $r->b = $r->a; + return $r; +} +var_dump(serialize(gen())); +?> +--EXPECTF-- +string(78) "O:8:"stdClass":2:{s:1:"a";a:1:{i:0;O:8:"stdClass":0:{}}s:1:"b";a:1:{i:0;r:3;}}" + + diff --git a/ext/standard/var.c b/ext/standard/var.c index c429763eb9c86..641e0275f5a03 100644 --- a/ext/standard/var.c +++ b/ext/standard/var.c @@ -666,8 +666,6 @@ static inline zend_long php_add_var_hash(php_serialize_data_t data, zval *var) / /* pass */ } else if (Z_TYPE_P(var) != IS_OBJECT) { return 0; - } else if (Z_REFCOUNT_P(var) == 1 && (Z_OBJ_P(var)->properties == NULL || GC_REFCOUNT(Z_OBJ_P(var)->properties) == 1)) { - return 0; } /* References to objects are treated as if the reference didn't exist */ From e374a03254f149efc0320563f0e73c7f9ada0c5d Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Tue, 30 May 2023 14:59:25 +0200 Subject: [PATCH 2/2] Skip serialize object recording when array paths are rc1 --- .../serialize/serialization_objects_019.phpt | 45 ++++++++++++++----- ext/standard/var.c | 36 +++++++++------ 2 files changed, 57 insertions(+), 24 deletions(-) diff --git a/ext/standard/tests/serialize/serialization_objects_019.phpt b/ext/standard/tests/serialize/serialization_objects_019.phpt index 505fc3d9af739..90d3056c5a6bd 100644 --- a/ext/standard/tests/serialize/serialization_objects_019.phpt +++ b/ext/standard/tests/serialize/serialization_objects_019.phpt @@ -2,16 +2,41 @@ Object serialization with references --FILE-- a = [$s]; - $r->b = $r->a; - return $r; + +function rcn() { + $root = new stdClass; + $end = new stdClass; + $root->a = [$end]; + $root->b = $root->a; + unset($end); + echo serialize($root), "\n"; } -var_dump(serialize(gen())); -?> ---EXPECTF-- -string(78) "O:8:"stdClass":2:{s:1:"a";a:1:{i:0;O:8:"stdClass":0:{}}s:1:"b";a:1:{i:0;r:3;}}" +function rcn_rc1() { + $root = new stdClass; + $end = new stdClass; + $root->a = [[$end]]; + $root->b = $root->a; + unset($end); + echo serialize($root), "\n"; +} +function rcn_properties_ht() { + $object = new stdClass; + $object->object = new stdClass; + $array = (array) $object; + $root = [$object, $array]; + unset($object); + unset($array); + echo serialize($root), "\n"; +} + +rcn(); +rcn_rc1(); +rcn_properties_ht(); + +?> +--EXPECT-- +O:8:"stdClass":2:{s:1:"a";a:1:{i:0;O:8:"stdClass":0:{}}s:1:"b";a:1:{i:0;r:3;}} +O:8:"stdClass":2:{s:1:"a";a:1:{i:0;a:1:{i:0;O:8:"stdClass":0:{}}}s:1:"b";a:1:{i:0;a:1:{i:0;r:4;}}} +a:2:{i:0;O:8:"stdClass":1:{s:6:"object";O:8:"stdClass":0:{}}i:1;a:1:{s:6:"object";r:3;}} diff --git a/ext/standard/var.c b/ext/standard/var.c index 641e0275f5a03..aab00e52d46dd 100644 --- a/ext/standard/var.c +++ b/ext/standard/var.c @@ -652,9 +652,12 @@ PHP_FUNCTION(var_export) } /* }}} */ -static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_data_t var_hash); +static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_data_t var_hash, bool in_rcn_array, bool is_root); -static inline zend_long php_add_var_hash(php_serialize_data_t data, zval *var) /* {{{ */ +/** + * @param bool in_rcn_array Whether the element appears in a potentially nested array with RC > 1. + */ +static inline zend_long php_add_var_hash(php_serialize_data_t data, zval *var, bool in_rcn_array) /* {{{ */ { zval *zv; zend_ulong key; @@ -666,6 +669,10 @@ static inline zend_long php_add_var_hash(php_serialize_data_t data, zval *var) / /* pass */ } else if (Z_TYPE_P(var) != IS_OBJECT) { return 0; + } else if (!in_rcn_array + && Z_REFCOUNT_P(var) == 1 + && (Z_OBJ_P(var)->properties == NULL || GC_REFCOUNT(Z_OBJ_P(var)->properties) == 1)) { + return 0; } /* References to objects are treated as if the reference didn't exist */ @@ -921,7 +928,7 @@ static int php_var_serialize_get_sleep_props( } /* }}} */ -static void php_var_serialize_nested_data(smart_str *buf, zval *struc, HashTable *ht, uint32_t count, bool incomplete_class, php_serialize_data_t var_hash) /* {{{ */ +static void php_var_serialize_nested_data(smart_str *buf, zval *struc, HashTable *ht, uint32_t count, bool incomplete_class, php_serialize_data_t var_hash, bool in_rcn_array) /* {{{ */ { smart_str_append_unsigned(buf, count); smart_str_appendl(buf, ":{", 2); @@ -951,19 +958,19 @@ static void php_var_serialize_nested_data(smart_str *buf, zval *struc, HashTable if (Z_TYPE_P(data) == IS_ARRAY) { if (UNEXPECTED(Z_IS_RECURSIVE_P(data)) || UNEXPECTED(Z_TYPE_P(struc) == IS_ARRAY && Z_ARR_P(data) == Z_ARR_P(struc))) { - php_add_var_hash(var_hash, struc); + php_add_var_hash(var_hash, struc, in_rcn_array); smart_str_appendl(buf, "N;", 2); } else { if (Z_REFCOUNTED_P(data)) { Z_PROTECT_RECURSION_P(data); } - php_var_serialize_intern(buf, data, var_hash); + php_var_serialize_intern(buf, data, var_hash, in_rcn_array, false); if (Z_REFCOUNTED_P(data)) { Z_UNPROTECT_RECURSION_P(data); } } } else { - php_var_serialize_intern(buf, data, var_hash); + php_var_serialize_intern(buf, data, var_hash, in_rcn_array, false); } } ZEND_HASH_FOREACH_END(); } @@ -978,13 +985,13 @@ static void php_var_serialize_class(smart_str *buf, zval *struc, HashTable *ht, if (php_var_serialize_get_sleep_props(&props, struc, ht) == SUCCESS) { php_var_serialize_class_name(buf, struc); php_var_serialize_nested_data( - buf, struc, &props, zend_hash_num_elements(&props), /* incomplete_class */ 0, var_hash); + buf, struc, &props, zend_hash_num_elements(&props), /* incomplete_class */ 0, var_hash, GC_REFCOUNT(&props) > 1); } zend_hash_destroy(&props); } /* }}} */ -static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_data_t var_hash) /* {{{ */ +static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_data_t var_hash, bool in_rcn_array, bool is_root) /* {{{ */ { zend_long var_already; HashTable *myht; @@ -993,7 +1000,7 @@ static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_ return; } - if (var_hash && (var_already = php_add_var_hash(var_hash, struc))) { + if (var_hash && (var_already = php_add_var_hash(var_hash, struc, in_rcn_array))) { if (var_already == -1) { /* Reference to an object that failed to serialize, replace with null. */ smart_str_appendl(buf, "N;", 2); @@ -1102,7 +1109,7 @@ static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_ if (Z_ISREF_P(data) && Z_REFCOUNT_P(data) == 1) { data = Z_REFVAL_P(data); } - php_var_serialize_intern(buf, data, var_hash); + php_var_serialize_intern(buf, data, var_hash, Z_REFCOUNT(retval) > 1, false); } ZEND_HASH_FOREACH_END(); smart_str_appendc(buf, '}'); @@ -1224,7 +1231,7 @@ static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_ prop = Z_REFVAL_P(prop); } - php_var_serialize_intern(buf, prop, var_hash); + php_var_serialize_intern(buf, prop, var_hash, false, false); } smart_str_appendc(buf, '}'); } else { @@ -1239,7 +1246,7 @@ static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_ if (count > 0 && incomplete_class) { --count; } - php_var_serialize_nested_data(buf, struc, myht, count, incomplete_class, var_hash); + php_var_serialize_nested_data(buf, struc, myht, count, incomplete_class, var_hash, GC_REFCOUNT(myht) > 1); zend_release_properties(myht); return; } @@ -1247,7 +1254,8 @@ static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_ smart_str_appendl(buf, "a:", 2); myht = Z_ARRVAL_P(struc); php_var_serialize_nested_data( - buf, struc, myht, zend_array_count(myht), /* incomplete_class */ 0, var_hash); + buf, struc, myht, zend_array_count(myht), /* incomplete_class */ 0, var_hash, + !is_root && (in_rcn_array || GC_REFCOUNT(myht) > 1)); return; case IS_REFERENCE: struc = Z_REFVAL_P(struc); @@ -1261,7 +1269,7 @@ static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_ PHPAPI void php_var_serialize(smart_str *buf, zval *struc, php_serialize_data_t *data) /* {{{ */ { - php_var_serialize_intern(buf, struc, *data); + php_var_serialize_intern(buf, struc, *data, false, true); smart_str_0(buf); } /* }}} */