From d9a35c7e97da5583c28d2799d64a6fccdf855622 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 29 May 2014 00:02:13 +0200 Subject: [PATCH] Fix class constant fetching If a class is extended after the constant fetch has been cached the cached value will be turned into a reference. On the next fetch the polymorphic cache will return this reference, which will be directly returned. The object assignment code then dereferences this result and performs a shallow copy, which is invalid for references. This subsequently leads to the constant value being prematurely freed. This is fixed by dereferencing the value returned from the polymorphic cache. Furthermore the incorrect dereference from in the object assignment code is replaced with an assertion that we're dealing with a non-reference, so ensure that this kind of problem cannot go unnoticed in the future. --- .../class_constant_to_reference_cached.phpt | 31 +++++++++++++++++++ Zend/zend_execute.c | 2 +- Zend/zend_vm_def.h | 5 ++- Zend/zend_vm_execute.h | 15 ++++----- 4 files changed, 40 insertions(+), 13 deletions(-) create mode 100644 Zend/tests/class_constant_to_reference_cached.phpt diff --git a/Zend/tests/class_constant_to_reference_cached.phpt b/Zend/tests/class_constant_to_reference_cached.phpt new file mode 100644 index 0000000000..b752226aba --- /dev/null +++ b/Zend/tests/class_constant_to_reference_cached.phpt @@ -0,0 +1,31 @@ +--TEST-- +Conversion of a class constant to a reference after it has been cached +--FILE-- +prop = self::TEST; + } +} + +$obj = new Test; +$obj->readConst(); +unset($obj); +var_dump(Test::TEST); + +eval('class Test2 extends Test {}'); + +$obj = new Test; +$obj->readConst(); +unset($obj); +var_dump(Test::TEST); + +?> +--EXPECT-- +string(4) "TEST" +string(4) "TEST" diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index e0653f9b63..9cc0f3a4cf 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -724,7 +724,7 @@ static inline void zend_assign_to_object(zval *retval, zval *object_ptr, zval *p /* separate our value if necessary */ if (value_type == IS_TMP_VAR) { - ZVAL_DEREF(value); + ZEND_ASSERT(Z_TYPE_P(value) != IS_REFERENCE); ZVAL_COPY_VALUE(&tmp, value); value = &tmp; } else if (value_type == IS_CONST) { diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index cba6e77d2b..5042c065ba 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -3631,15 +3631,14 @@ ZEND_VM_HANDLER(99, ZEND_FETCH_CONSTANT, VAR|CONST|UNUSED, CONST) } else { ce = Z_CE_P(EX_VAR(opline->op1.var)); if ((value = CACHED_POLYMORPHIC_PTR(Z_CACHE_SLOT_P(opline->op2.zv), ce)) != NULL) { + ZVAL_DEREF(value); ZVAL_DUP(EX_VAR(opline->result.var), value); goto constant_fetch_end; } } if (EXPECTED((value = zend_hash_find(&ce->constants_table, Z_STR_P(opline->op2.zv))) != NULL)) { - if (Z_ISREF_P(value)) { - value = Z_REFVAL_P(value); - } + ZVAL_DEREF(value); if (Z_CONSTANT_P(value)) { zend_class_entry *old_scope = EG(scope); diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index 2e7184e30d..5b5c69923a 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -3903,15 +3903,14 @@ static int ZEND_FASTCALL ZEND_FETCH_CONSTANT_SPEC_CONST_CONST_HANDLER(ZEND_OPCO } else { ce = Z_CE_P(EX_VAR(opline->op1.var)); if ((value = CACHED_POLYMORPHIC_PTR(Z_CACHE_SLOT_P(opline->op2.zv), ce)) != NULL) { + ZVAL_DEREF(value); ZVAL_DUP(EX_VAR(opline->result.var), value); goto constant_fetch_end; } } if (EXPECTED((value = zend_hash_find(&ce->constants_table, Z_STR_P(opline->op2.zv))) != NULL)) { - if (Z_ISREF_P(value)) { - value = Z_REFVAL_P(value); - } + ZVAL_DEREF(value); if (Z_CONSTANT_P(value)) { zend_class_entry *old_scope = EG(scope); @@ -15346,15 +15345,14 @@ static int ZEND_FASTCALL ZEND_FETCH_CONSTANT_SPEC_VAR_CONST_HANDLER(ZEND_OPCODE } else { ce = Z_CE_P(EX_VAR(opline->op1.var)); if ((value = CACHED_POLYMORPHIC_PTR(Z_CACHE_SLOT_P(opline->op2.zv), ce)) != NULL) { + ZVAL_DEREF(value); ZVAL_DUP(EX_VAR(opline->result.var), value); goto constant_fetch_end; } } if (EXPECTED((value = zend_hash_find(&ce->constants_table, Z_STR_P(opline->op2.zv))) != NULL)) { - if (Z_ISREF_P(value)) { - value = Z_REFVAL_P(value); - } + ZVAL_DEREF(value); if (Z_CONSTANT_P(value)) { zend_class_entry *old_scope = EG(scope); @@ -24557,15 +24555,14 @@ static int ZEND_FASTCALL ZEND_FETCH_CONSTANT_SPEC_UNUSED_CONST_HANDLER(ZEND_OPC } else { ce = Z_CE_P(EX_VAR(opline->op1.var)); if ((value = CACHED_POLYMORPHIC_PTR(Z_CACHE_SLOT_P(opline->op2.zv), ce)) != NULL) { + ZVAL_DEREF(value); ZVAL_DUP(EX_VAR(opline->result.var), value); goto constant_fetch_end; } } if (EXPECTED((value = zend_hash_find(&ce->constants_table, Z_STR_P(opline->op2.zv))) != NULL)) { - if (Z_ISREF_P(value)) { - value = Z_REFVAL_P(value); - } + ZVAL_DEREF(value); if (Z_CONSTANT_P(value)) { zend_class_entry *old_scope = EG(scope); -- 2.50.1