php8.6: Fix set visibility bypass through a primed ZEND_ASSIGN_OBJ cache slot

NEWS

@@ -2,6 +2,10 @@ PHP                                                                        NEWS
 |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 ?? ??? ????, PHP 8.6.0RC5
 
+- Core:
+  . Fixed set visibility bypass through a primed ZEND_ASSIGN_OBJ cache slot.
+    (Matthias Dötsch)
+
 - Opcache:
   . Fixed tracing JIT returning a wrapped integer instead of a float on
     integer overflow after an integer was compared with a float or

Zend/tests/asymmetric_visibility/cache_slot_magic_set.phpt

@@ -0,0 +1,28 @@
+--TEST--
+Asymmetric visibility: cache slot primed by a __set() fallback must not skip the set visibility check
+--FILE--
+<?php
+class C {
+    public private(set) int $x = 0;
+    public function __set($name, $value) { echo "__set($name)\n"; }
+    public function unsetX() { unset($this->x); }
+}
+function w(C $c, int $v) {
+    try {
+        $c->x = $v;   // same opline every call
+    } catch (Error $e) {
+        echo $e->getMessage(), "\n";
+    }
+}
+$a = new C();
+$a->unsetX();
+w($a, 1);         // unset property: goes to __set(), cache slot stays primed
+$b = new C();
+w($b, 2);         // initialized property, must fail: private(set) from global scope
+var_dump($b->x);
+
+?>
+--EXPECT--
+__set(x)
+Cannot modify private(set) property C::$x from global scope
+int(0)

Zend/tests/readonly_props/cache_slot_clone_reinit.phpt

@@ -0,0 +1,26 @@
+--TEST--
+Readonly: cache slot primed by a failed modification must not skip the set visibility check on reinitialization
+--FILE--
+<?php
+class P {
+    public readonly int $x;
+    public function __construct() { $this->x = 1; }
+    public function __clone() { w($this, 3); }
+}
+function w(P $p, int $v) {
+    try {
+        $p->x = $v;   // same opline every call
+    } catch (Error $e) {
+        echo $e->getMessage(), "\n";
+    }
+}
+$p = new P();
+w($p, 2);        // readonly modification error, cache slot stays primed
+$q = clone $p;   // __clone -> w(): global scope must not be able to reinitialize
+var_dump($q->x);
+
+?>
+--EXPECT--
+Cannot modify readonly property P::$x
+Cannot modify protected(set) readonly property P::$x from global scope
+int(1)

Zend/zend_object_handlers.c

@@ -1059,6 +1059,16 @@ ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int
 }
 /* }}} */
 
+/* The ZEND_ASSIGN_OBJ fast path skips the set visibility check when the cache slot is
+ * primed, assuming write_property() only primes it after that check passed. Reset the
+ * slot on the paths that leave it primed without having checked. */
+static zend_always_inline void zend_reset_property_cache_slot(void **cache_slot) {
+	if (cache_slot) {
+		CACHE_POLYMORPHIC_PTR_EX(cache_slot, NULL, NULL);
+		CACHE_PTR_EX(cache_slot + 2, NULL);
+	}
+}
+
 static zend_always_inline bool property_uses_strict_types(void) {
 	const zend_execute_data *execute_data = EG(current_execute_data);
 	return execute_data
@@ -1131,18 +1141,19 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva
 				 && !(Z_PROP_FLAG_P(variable_ptr) & IS_PROP_REINITABLE)) {
 					zend_readonly_property_modification_error(prop_info);
 					variable_ptr = &EG(error_zval);
+					/* The set visibility was not checked. */
+					zend_reset_property_cache_slot(cache_slot);
 					goto exit;
 				}
 				if ((prop_info->flags & ZEND_ACC_PPP_SET_MASK) && !zend_asymmetric_property_has_set_access(prop_info)) {
 					zend_asymmetric_visibility_property_modification_error(prop_info, "modify");
 					variable_ptr = &EG(error_zval);
-					if (cache_slot) {
-						/* Reset cache slot to dodge fast path in next execution. */
-						CACHE_POLYMORPHIC_PTR_EX(cache_slot, NULL, NULL);
-						CACHE_PTR_EX(cache_slot + 2, NULL);
-					}
+					zend_reset_property_cache_slot(cache_slot);
 					goto exit;
 				}
+			} else {
+				/* __set() will be called without checking the set visibility. */
+				zend_reset_property_cache_slot(cache_slot);
 			}
 		}