Repository navigation
Assertion `zval_get_type(&(*(zptr))) == 6 && "Concat should return string"' failed #10571
Description
Description
The following code:
<?php
class A
{
public string $prop = "";
}
class B
{
public function __toString()
{
global $a;
$a = $a = "";
$a->p = "";
return "";
}
}
$a = new A();
$a = $a->prop .= new B();
?>Resulted in this output:
php-src/Zend/zend_execute.c:1582: zend_binary_assign_op_typed_prop: Assertion `zval_get_type(&(*(zptr))) == 6 && "Concat should return string"' failed.
With asan, the output is:
==5503==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000001 (pc 0x555555ef095f bp 0x7fffffff9b00 sp 0x7fffffff9ad0 T0)
==5503==The signal is caused by a WRITE memory access.
==5503==Hint: address points to the zero page.
#0 0x555555ef095e in zend_gc_addref /php-src/Zend/zend_types.h:1210
#1 0x555555ef095e in ZEND_ASSIGN_OBJ_OP_SPEC_CV_CONST_HANDLER /php-src/Zend/zend_vm_execute.h:40931
#2 0x555555f5c7cf in execute_ex /php-src/Zend/zend_vm_execute.h:60234
#3 0x555555f91637 in zend_execute /php-src/Zend/zend_vm_execute.h:61087
...)
It seems a use-after-free bug.
PHP Version
PHP 8.3.0-dev
Operating System
No response
Activity
ndossche commentedon Feb 12, 2023
Looks like a variant of #10169. And therefore is related to PR #10546.
iluuu1994 commentedon Feb 12, 2023
I think this is related to #10179, rather. Unfortunately that fix did not cover this problem.
iluuu1994 commentedon Feb 12, 2023
Taking a quick look, it seems we'd need to increase the refcount of the object (that's not currently passed, refs might need special handling) in
zend_binary_assign_op_typed_prop. As usual, this is unfortunate because it's not usually necessary. AVARcannot be released, neither can aCVbecause it's not possible to access other scopes. The only exception are globals...Edit: Ugh. Furthermore releasing the object in
zend_binary_assign_op_typed_propwould once again cause issues withZVAL_COPY(EX_VAR(opline->result.var), zptr);in the VM. This is a mess...ndossche commentedon Feb 12, 2023
There needs to be a fundamental change to fix this in all cases, preferably in a uniform way. We could try patching it up ad-hoc for 8.1 and 8.2 and fix it better (in a different way) with a small BC break in 8.3 by delaying destructors uniformly until after the copies.
Edit: edited for clarity