diff --git a/Zend/tests/gh16646.phpt b/Zend/tests/gh16646.phpt new file mode 100644 index 0000000000000..b6cb503d8ed05 --- /dev/null +++ b/Zend/tests/gh16646.phpt @@ -0,0 +1,32 @@ +--TEST-- +GH-16646: Use-after-free in ArrayObject::unset() with destructor +--FILE-- +b = $arg; + } +} + +class C { + function __destruct() { + global $arr; + echo __METHOD__, "\n"; + $arr->exchangeArray([]); + } +} + +$arr = new ArrayObject(new B(new C)); +unset($arr["b"]); +var_dump($arr); + +?> +--EXPECT-- +C::__destruct +object(ArrayObject)#1 (1) { + ["storage":"ArrayObject":private]=> + array(0) { + } +} diff --git a/Zend/tests/gh16646_2.phpt b/Zend/tests/gh16646_2.phpt new file mode 100644 index 0000000000000..d006583500811 --- /dev/null +++ b/Zend/tests/gh16646_2.phpt @@ -0,0 +1,25 @@ +--TEST-- +GH-16646: Use-after-free in ArrayObject::exchangeArray() with destructor +--FILE-- +exchangeArray([]); + } +} + +$arr = new ArrayObject(new C); +$arr->exchangeArray([]); +var_dump($arr); + +?> +--EXPECT-- +C::__destruct +object(ArrayObject)#1 (1) { + ["storage":"ArrayObject":private]=> + array(0) { + } +} diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index dbe110f7c40da..2a721d79dbc55 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -555,13 +555,15 @@ static void spl_array_unset_dimension_ex(int check_inherited, zend_object *objec if (Z_TYPE_P(data) == IS_INDIRECT) { data = Z_INDIRECT_P(data); if (Z_TYPE_P(data) != IS_UNDEF) { - zval_ptr_dtor(data); + zval garbage; + ZVAL_COPY_VALUE(&garbage, data); ZVAL_UNDEF(data); HT_FLAGS(ht) |= HASH_FLAG_HAS_EMPTY_IND; zend_hash_move_forward_ex(ht, spl_array_get_pos_ptr(ht, intern)); if (spl_array_is_object(intern)) { spl_array_skip_protected(intern, ht); } + zval_ptr_dtor(&garbage); } } else { zend_hash_del(ht, key.key); @@ -1052,8 +1054,10 @@ static HashTable *spl_array_it_get_gc(zend_object_iterator *iter, zval **table, static void spl_array_set_array(zval *object, spl_array_object *intern, zval *array, zend_long ar_flags, bool just_array) { /* Handled by ZPP prior to this, or for __unserialize() before passing to here */ ZEND_ASSERT(Z_TYPE_P(array) == IS_ARRAY || Z_TYPE_P(array) == IS_OBJECT); + zval garbage; + ZVAL_UNDEF(&garbage); if (Z_TYPE_P(array) == IS_ARRAY) { - zval_ptr_dtor(&intern->array); + ZVAL_COPY_VALUE(&garbage, &intern->array); if (Z_REFCOUNT_P(array) == 1) { ZVAL_COPY(&intern->array, array); } else { @@ -1071,7 +1075,7 @@ static void spl_array_set_array(zval *object, spl_array_object *intern, zval *ar } } else { if (Z_OBJ_HT_P(array) == &spl_handler_ArrayObject || Z_OBJ_HT_P(array) == &spl_handler_ArrayIterator) { - zval_ptr_dtor(&intern->array); + ZVAL_COPY_VALUE(&garbage, &intern->array); if (just_array) { spl_array_object *other = Z_SPLARRAY_P(array); ar_flags = other->ar_flags & ~SPL_ARRAY_INT_MASK; @@ -1089,9 +1093,10 @@ static void spl_array_set_array(zval *object, spl_array_object *intern, zval *ar zend_throw_exception_ex(spl_ce_InvalidArgumentException, 0, "Overloaded object of type %s is not compatible with %s", ZSTR_VAL(Z_OBJCE_P(array)->name), ZSTR_VAL(intern->std.ce->name)); + ZEND_ASSERT(Z_TYPE(garbage) == IS_UNDEF); return; } - zval_ptr_dtor(&intern->array); + ZVAL_COPY_VALUE(&garbage, &intern->array); ZVAL_COPY(&intern->array, array); } } @@ -1102,6 +1107,8 @@ static void spl_array_set_array(zval *object, spl_array_object *intern, zval *ar zend_hash_iterator_del(intern->ht_iter); intern->ht_iter = (uint32_t)-1; } + + zval_ptr_dtor(&garbage); } /* }}} */ diff --git a/ext/spl/tests/gh16646.phpt b/ext/spl/tests/gh16646.phpt new file mode 100644 index 0000000000000..003827e432c72 --- /dev/null +++ b/ext/spl/tests/gh16646.phpt @@ -0,0 +1,32 @@ +--TEST-- +GH-16646: Test +--FILE-- +b = $arg; + } +} + +class C { + function __destruct() { + global $arr; + echo __METHOD__, "\n"; + $arr->exchangeArray([]); + } +} + +$arr = new ArrayObject(new B(new C)); +unset($arr["b"]); +var_dump($arr); + +?> +--EXPECT-- +C::__destruct +object(ArrayObject)#1 (1) { + ["storage":"ArrayObject":private]=> + array(0) { + } +}