diff --git a/Zend/tests/gh19305-001.phpt b/Zend/tests/gh19305-001.phpt new file mode 100644 index 0000000000000..4c9ee37473e34 --- /dev/null +++ b/Zend/tests/gh19305-001.phpt @@ -0,0 +1,27 @@ +--TEST-- +GH-19305 001: Operands may be released during comparison +--FILE-- + 'test', + 'bar' => 2, +]; +$b = (object)[ + 'foo' => new class { + public function __toString() { + global $a, $b; + $a = $b = null; + return ''; + } + }, + 'bar' => 2, +]; + +// Comparison of $a->foo and $b->foo calls __toString(), which releases +// both $a and $b. +var_dump($a > $b); + +?> +--EXPECT-- +bool(true) diff --git a/Zend/tests/gh19305-002.phpt b/Zend/tests/gh19305-002.phpt new file mode 100644 index 0000000000000..790156191317b --- /dev/null +++ b/Zend/tests/gh19305-002.phpt @@ -0,0 +1,27 @@ +--TEST-- +GH-19305 002: Operands may be released during comparison +--FILE-- + 'test', + 'bar' => 2, +]; +$b = [ + 'foo' => new class { + public function __toString() { + global $a, $b; + $a = $b = null; + return ''; + } + }, + 'bar' => 2, +]; + +// Comparison of $a['foo'] and $b['foo'] calls __toString(), which releases +// both $a and $b. +var_dump($a > $b); + +?> +--EXPECT-- +bool(true) diff --git a/Zend/tests/gh19305-003.phpt b/Zend/tests/gh19305-003.phpt new file mode 100644 index 0000000000000..7b071156ef8e3 --- /dev/null +++ b/Zend/tests/gh19305-003.phpt @@ -0,0 +1,28 @@ +--TEST-- +GH-19305 003: Operands may be released during comparison +--SKIPIF-- + +--FILE-- +newLazyProxy(function () { return new C; }); + +// Comparison calls initializers, which releases $o +var_dump($o > +$r->newLazyGhost(function () { + global $o; + $o = null; +})); + +?> +--EXPECT-- +bool(false) diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index 180364b248d71..2eacb614f97c4 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -1792,6 +1792,10 @@ ZEND_API int zend_std_compare_objects(zval *o1, zval *o2) /* {{{ */ } Z_PROTECT_RECURSION_P(o1); + GC_ADDREF(zobj1); + GC_ADDREF(zobj2); + int ret; + for (i = 0; i < zobj1->ce->default_properties_count; i++) { zval *p1, *p2; @@ -1806,35 +1810,50 @@ ZEND_API int zend_std_compare_objects(zval *o1, zval *o2) /* {{{ */ if (Z_TYPE_P(p1) != IS_UNDEF) { if (Z_TYPE_P(p2) != IS_UNDEF) { - int ret; - ret = zend_compare(p1, p2); if (ret != 0) { Z_UNPROTECT_RECURSION_P(o1); - return ret; + goto done; } } else { Z_UNPROTECT_RECURSION_P(o1); - return 1; + ret = 1; + goto done; } } else { if (Z_TYPE_P(p2) != IS_UNDEF) { Z_UNPROTECT_RECURSION_P(o1); - return 1; + ret = 1; + goto done; } } } Z_UNPROTECT_RECURSION_P(o1); - return 0; + ret = 0; + +done: + OBJ_RELEASE(zobj1); + OBJ_RELEASE(zobj2); + + return ret; } else { + GC_ADDREF(zobj1); + GC_ADDREF(zobj2); + if (!zobj1->properties) { rebuild_object_properties(zobj1); } if (!zobj2->properties) { rebuild_object_properties(zobj2); } - return zend_compare_symbol_tables(zobj1->properties, zobj2->properties); + + int ret = zend_compare_symbol_tables(zobj1->properties, zobj2->properties); + + OBJ_RELEASE(zobj1); + OBJ_RELEASE(zobj2); + + return ret; } } /* }}} */ diff --git a/Zend/zend_operators.c b/Zend/zend_operators.c index 38c87dfe98dbd..8646fc25be260 100644 --- a/Zend/zend_operators.c +++ b/Zend/zend_operators.c @@ -3384,7 +3384,19 @@ static int hash_zval_compare_function(zval *z1, zval *z2) /* {{{ */ ZEND_API int ZEND_FASTCALL zend_compare_symbol_tables(HashTable *ht1, HashTable *ht2) /* {{{ */ { - return ht1 == ht2 ? 0 : zend_hash_compare(ht1, ht2, (compare_func_t) hash_zval_compare_function, 0); + if (ht1 == ht2) { + return 0; + } + + GC_TRY_ADDREF(ht1); + GC_TRY_ADDREF(ht2); + + int ret = zend_hash_compare(ht1, ht2, (compare_func_t) hash_zval_compare_function, 0); + + GC_TRY_DTOR_NO_REF(ht1); + GC_TRY_DTOR_NO_REF(ht2); + + return ret; } /* }}} */ diff --git a/Zend/zend_types.h b/Zend/zend_types.h index c4a07f58874ab..63f6ddbc97242 100644 --- a/Zend/zend_types.h +++ b/Zend/zend_types.h @@ -731,6 +731,18 @@ static zend_always_inline uint8_t zval_get_type(const zval* pz) { } \ } while (0) +#define GC_TRY_DTOR_NO_REF(p) \ + do { \ + zend_refcounted_h *_p = &(p)->gc; \ + if (!(_p->u.type_info & GC_IMMUTABLE)) { \ + if (zend_gc_delref(_p) == 0) { \ + rc_dtor_func((zend_refcounted *)_p); \ + } else { \ + gc_check_possible_root_no_ref((zend_refcounted *)_p); \ + } \ + } \ + } while (0) + #define GC_TYPE_MASK 0x0000000f #define GC_FLAGS_MASK 0x000003f0 #define GC_INFO_MASK 0xfffffc00