Merge branch 'PHP-8.4'

* PHP-8.4:
  Prevent operands from being released during comparison

Author: arnaud-lb

NEWS

@@ -2,6 +2,10 @@ PHP                                                                        NEWS
 |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 ?? ??? ????, PHP 8.5.0beta1
 
+- Core:
+  . Fixed bug GH-19305 (Operands may be being released during comparison).
+    (Arnaud)
+
 - Opcache:
   . Fixed bug GH-19301 (opcache build failure). (Remi)
 

Zend/tests/gh19305-001.phpt

@@ -0,0 +1,27 @@
+--TEST--
+GH-19305 001: Operands may be released during comparison
+--FILE--
+<?php
+
+$a = (object)[
+    'foo' => '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)

Zend/tests/gh19305-002.phpt

@@ -0,0 +1,27 @@
+--TEST--
+GH-19305 002: Operands may be released during comparison
+--FILE--
+<?php
+
+$a = [
+    'foo' => '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)

Zend/tests/gh19305-003.phpt

@@ -0,0 +1,28 @@
+--TEST--
+GH-19305 003: Operands may be released during comparison
+--SKIPIF--
+<?php
+if (!method_exists('ReflectionClass', 'newLazyGhost')) {
+    die('skip No lazy objects');
+}
+?>
+--FILE--
+<?php
+
+class C
+{
+    public $s;
+}
+$r = new ReflectionClass(C::class);
+$o = $r->newLazyProxy(function () { return new C; });
+
+// Comparison calls initializers, which releases $o
+var_dump($o >
+$r->newLazyGhost(function () {
+    global $o;
+    $o = null;
+}));
+
+?>
+--EXPECT--
+bool(false)

Zend/zend_object_handlers.c

@@ -2204,6 +2204,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;
 
@@ -2218,31 +2222,45 @@ 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 {
-		return zend_compare_symbol_tables(
+		GC_ADDREF(zobj1);
+		GC_ADDREF(zobj2);
+
+		int ret = zend_compare_symbol_tables(
 				zend_std_get_properties_ex(zobj1),
 				zend_std_get_properties_ex(zobj2));
+
+		OBJ_RELEASE(zobj1);
+		OBJ_RELEASE(zobj2);
+
+		return ret;
 	}
 }
 /* }}} */

Zend/zend_operators.c

@@ -3422,7 +3422,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;
 }
 /* }}} */
 

Zend/zend_types.h

@@ -753,6 +753,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