Lazy object revert init with object destructor can lead to double free

[iluuu1994]

Description

Originally reported by @cnwangjihe.

The following code:

<?php

class A {
    public $b;
}

class B {
    public function __construct(public $a) {}
    public function __destruct() {
        unset($this->a->b);
    }
}

$r = new ReflectionClass(A::class);

$a = $r->newLazyGhost(function (A $a) {
    $a->b = new B($a);
    throw new Exception();
});

$a->any;

Resulted in this output:

=================================================================
==86350==ERROR: AddressSanitizer: heap-use-after-free on address 0x6ca445bf6820 at pc 0x5b90bfecc629 bp 0x7fff60ef9820 sp 0x7fff60ef9818
READ of size 4 at 0x6ca445bf6820 thread T0
    #0 0x5b90bfecc628 in zend_gc_delref /home/ilutov/Developer/php-src/Zend/zend_types.h:809
    #1 0x5b90bfecee85 in zend_objects_store_del /home/ilutov/Developer/php-src/Zend/zend_objects_API.c:179
    #2 0x5b90bff640be in rc_dtor_func /home/ilutov/Developer/php-src/Zend/zend_variables.c:56
    #3 0x5b90bfed1701 in i_zval_ptr_dtor /home/ilutov/Developer/php-src/Zend/zend_variables.h:44
    #4 0x5b90bfed33e5 in zend_object_dtor_property /home/ilutov/Developer/php-src/Zend/zend_objects.c:72
    #5 0x5b90bfe9092f in zend_lazy_object_revert_init /home/ilutov/Developer/php-src/Zend/zend_lazy_objects.c:419
    #6 0x5b90bfe94ecb in zend_lazy_object_init /home/ilutov/Developer/php-src/Zend/zend_lazy_objects.c:664
    #7 0x5b90bfeb1e70 in zend_std_read_property /home/ilutov/Developer/php-src/Zend/zend_object_handlers.c:986
    #8 0x5b90bfc57db8 in ZEND_FETCH_OBJ_R_SPEC_CV_CONST_INLINE_HANDLER /home/ilutov/Developer/php-src/Zend/zend_vm_execute.h:42312
    #9 0x5b90bfcf8b15 in execute_ex /home/ilutov/Developer/php-src/Zend/zend_vm_execute.h:114704
    #10 0x5b90bfcfdca2 in zend_execute /home/ilutov/Developer/php-src/Zend/zend_vm_execute.h:115646
    #11 0x5b90bff8ea2f in zend_execute_script /home/ilutov/Developer/php-src/Zend/zend.c:1972
    #12 0x5b90bf487e7b in php_execute_script_ex /home/ilutov/Developer/php-src/main/main.c:2655
    #13 0x5b90bf488528 in php_execute_script /home/ilutov/Developer/php-src/main/main.c:2695
    #14 0x5b90bff97072 in do_cli /home/ilutov/Developer/php-src/sapi/cli/php_cli.c:947
    #15 0x5b90bff9adfc in main /home/ilutov/Developer/php-src/sapi/cli/php_cli.c:1370

But I expected this output instead:

What happens is roughly:

• The initializer is called.
• $a and $b form a cycle.
• The initializer throws, starting the reverting process.
• The new value of the property $b is destroyed.
• Being the only reference, B::__destruct() is called. Before that, the refcount of $b is set to 1.
• $a->b has not been updated yet, so unset($this->a->b); can decrement $b's refcount again.
• It reaches 0 again, skipping the destructor that has been called already, and freeing $b.
• When we exit and come back to the destructor path, accessing $b triggers a UAF.

Naive fix (can likely be restricted to just lazy objects):

diff --git a/Zend/zend_objects.c b/Zend/zend_objects.c
index 2fc264742cd..991bffc8fc4 100644
--- a/Zend/zend_objects.c
+++ b/Zend/zend_objects.c
@@ -69,7 +69,7 @@ void zend_object_dtor_property(zend_object *object, zval *p)
 				ZEND_REF_DEL_TYPE_SOURCE(Z_REF_P(p), prop_info);
 			}
 		}
-		i_zval_ptr_dtor(p);
+		zval_ptr_safe_dtor(p);
 	}
 }

PHP Version

PHP 8.4+

Operating System

No response