Skip to content

Commit c70f217

Browse files
committed
prepare: don't bother making another property info copy if it already exists
fo classes that were unlinked during first copy, we might've already copied the relevant property info to the destination thread. In this case in the past, this would just silently fail to insert the updated property info and go on to free it. We probably ought to get rid of the old property_info and replace it with the newly copied version, but for now this restores the original behaviour, which seemed to work anyway??? However, possible that this may break with property hooks inheritance. Needs further testing.
1 parent 92ebc57 commit c70f217

File tree

1 file changed

+9
-4
lines changed

1 file changed

+9
-4
lines changed

src/prepare.c

+9-4
Original file line numberDiff line numberDiff line change
@@ -255,10 +255,15 @@ static void prepare_class_property_table(const pmmpthread_ident_t* source, zend_
255255
zend_property_info *info;
256256
zend_string *name;
257257
ZEND_HASH_FOREACH_STR_KEY_PTR(&candidate->properties_info, name, info) {
258-
zend_property_info* dup = copy_property_info(source, candidate, prepared, info);
259-
if (!zend_hash_str_add_ptr(&prepared->properties_info, name->val, name->len, dup)) {
260-
if (dup->doc_comment)
261-
zend_string_release(dup->doc_comment);
258+
zend_property_info* dup = zend_hash_find_ptr(&prepared->properties_info, name);
259+
//TODO: if this is non-null it may need updating (if we copied it previously for an unlinked class)
260+
//for now this just ensures that we don't have UAFs with reused property infos
261+
//hopefully this doesn't shit a brick???
262+
if (dup == NULL) {
263+
dup = copy_property_info(source, candidate, prepared, info);
264+
if (!zend_hash_str_add_ptr(&prepared->properties_info, name->val, name->len, dup)) {
265+
ZEND_ASSERT(0);
266+
}
262267
}
263268
} ZEND_HASH_FOREACH_END();
264269

0 commit comments

Comments
 (0)