Fix ReflectionProperty::getRawValue() and related methods for properties overridden with hooks

`new Reflectionproperty($scope, $propName)` keeps a reference to the
zend_property_info of $propName declared in $scope. In getRawValue() and
related methods, we use this reference to check whether the property is hooked.

Calling `new ReflectionProperty($scope, $propName)->getRawValue($object)` is
equivalent to the expression $object->$propName from scope $scope (except that
it bypasses hooks), and thus may access an overridden property (unless the
original is private). This property may have hooks and different flags.

Here I fetch the effective property info before checking for hooks and
property flags.

Fixes GH-17713
Closes GH-17714

NEWS

@@ -15,6 +15,8 @@ PHP                                                                        NEWS
     by reference). (ilutov)
   . Fixed bug GH-17718 (Calling static methods on an interface that has
     `__callStatic` is allowed). (timwolla)
+  . Fixed bug GH-17713 (ReflectionProperty::getRawValue() and related methods
+    may call hooks of overridden properties). (Arnaud)
 
 - DOM:
   . Fixed bug GH-17609 (Typo in error message: Dom\NO_DEFAULT_NS instead of

Zend/tests/lazy_objects/skipLazyInitialization.phpt

@@ -198,10 +198,10 @@ getValue(): string(5) "value"
 ## Property [ public static $static = 'static' ]
 
 skipInitializerForProperty():
-ReflectionException: Can not use skipLazyInitialization on static property B::$static
+ReflectionException: Can not use skipLazyInitialization on static property A::$static
 
 setRawValueWithoutLazyInitialization():
-ReflectionException: Can not use setRawValueWithoutLazyInitialization on static property B::$static
+ReflectionException: Can not use setRawValueWithoutLazyInitialization on static property A::$static
 
 ## Property [ public $noDefault = NULL ]
 
@@ -238,10 +238,10 @@ getValue(): string(5) "value"
 ## Property [ public $virtual ]
 
 skipInitializerForProperty():
-ReflectionException: Can not use skipLazyInitialization on virtual property B::$virtual
+ReflectionException: Can not use skipLazyInitialization on virtual property A::$virtual
 
 setRawValueWithoutLazyInitialization():
-ReflectionException: Can not use setRawValueWithoutLazyInitialization on virtual property B::$virtual
+ReflectionException: Can not use setRawValueWithoutLazyInitialization on virtual property A::$virtual
 
 ## Property [ $dynamicProp ]
 
@@ -295,10 +295,10 @@ getValue(): string(5) "value"
 ## Property [ public static $static = 'static' ]
 
 skipInitializerForProperty():
-ReflectionException: Can not use skipLazyInitialization on static property B::$static
+ReflectionException: Can not use skipLazyInitialization on static property A::$static
 
 setRawValueWithoutLazyInitialization():
-ReflectionException: Can not use setRawValueWithoutLazyInitialization on static property B::$static
+ReflectionException: Can not use setRawValueWithoutLazyInitialization on static property A::$static
 
 ## Property [ public $noDefault = NULL ]
 
@@ -335,10 +335,10 @@ getValue(): string(5) "value"
 ## Property [ public $virtual ]
 
 skipInitializerForProperty():
-ReflectionException: Can not use skipLazyInitialization on virtual property B::$virtual
+ReflectionException: Can not use skipLazyInitialization on virtual property A::$virtual
 
 setRawValueWithoutLazyInitialization():
-ReflectionException: Can not use setRawValueWithoutLazyInitialization on virtual property B::$virtual
+ReflectionException: Can not use setRawValueWithoutLazyInitialization on virtual property A::$virtual
 
 ## Property [ $dynamicProp ]
 

ext/reflection/php_reflection.c

@@ -6096,6 +6096,19 @@ ZEND_METHOD(ReflectionProperty, setValue)
 }
 /* }}} */
 
+/* Return the property info being used when accessing 'ref->prop' from scope
+ * 'scope' on 'object'. The result may be different from 'ref->prop' when the
+ * property is overridden on 'object' and was not private in 'scope'.
+ * The effective prop may add hooks or change flags. */
+static zend_property_info *reflection_property_get_effective_prop(
+		property_reference *ref, zend_class_entry *scope, zend_object *object) {
+	zend_property_info *prop = ref->prop;
+	if (scope != object->ce && !(prop && (prop->flags & ZEND_ACC_PRIVATE))) {
+		prop = zend_hash_find_ptr(&object->ce->properties_info, ref->unmangled_name);
+	}
+	return prop;
+}
+
 ZEND_METHOD(ReflectionProperty, getRawValue)
 {
 	reflection_object *intern;
@@ -6108,17 +6121,20 @@ ZEND_METHOD(ReflectionProperty, getRawValue)
 
 	GET_REFLECTION_OBJECT_PTR(ref);
 
-	if (prop_get_flags(ref) & ZEND_ACC_STATIC) {
-		_DO_THROW("May not use getRawValue on static properties");
-		RETURN_THROWS();
-	}
-
 	if (!instanceof_function(Z_OBJCE_P(object), intern->ce)) {
 		_DO_THROW("Given object is not an instance of the class this property was declared in");
 		RETURN_THROWS();
 	}
 
-	if (!ref->prop || !ref->prop->hooks || !ref->prop->hooks[ZEND_PROPERTY_HOOK_GET]) {
+	zend_property_info *prop = reflection_property_get_effective_prop(ref,
+			intern->ce, Z_OBJ_P(object));
+
+	if (UNEXPECTED(prop && (prop->flags & ZEND_ACC_STATIC))) {
+		_DO_THROW("May not use getRawValue on static properties");
+		RETURN_THROWS();
+	}
+
+	if (!prop || !prop->hooks || !prop->hooks[ZEND_PROPERTY_HOOK_GET]) {
 		zval rv;
 		zval *member_p = zend_read_property_ex(intern->ce, Z_OBJ_P(object), ref->unmangled_name, 0, &rv);
 
@@ -6131,17 +6147,19 @@ ZEND_METHOD(ReflectionProperty, getRawValue)
 			RETURN_COPY_VALUE(member_p);
 		}
 	} else {
-		zend_function *func = zend_get_property_hook_trampoline(ref->prop, ZEND_PROPERTY_HOOK_GET, ref->unmangled_name);
+		zend_function *func = zend_get_property_hook_trampoline(prop, ZEND_PROPERTY_HOOK_GET, ref->unmangled_name);
 		zend_call_known_instance_method_with_0_params(func, Z_OBJ_P(object), return_value);
 	}
 }
 
-static void reflection_property_set_raw_value(property_reference *ref, reflection_object *intern, zend_object *object, zval *value)
+static void reflection_property_set_raw_value(zend_property_info *prop,
+		zend_string *unmangled_name, reflection_object *intern,
+		zend_object *object, zval *value)
 {
-	if (!ref->prop || !ref->prop->hooks || !ref->prop->hooks[ZEND_PROPERTY_HOOK_SET]) {
-		zend_update_property_ex(intern->ce, object, ref->unmangled_name, value);
+	if (!prop || !prop->hooks || !prop->hooks[ZEND_PROPERTY_HOOK_SET]) {
+		zend_update_property_ex(intern->ce, object, unmangled_name, value);
 	} else {
-		zend_function *func = zend_get_property_hook_trampoline(ref->prop, ZEND_PROPERTY_HOOK_SET, ref->unmangled_name);
+		zend_function *func = zend_get_property_hook_trampoline(prop, ZEND_PROPERTY_HOOK_SET, unmangled_name);
 		zend_call_known_instance_method_with_1_params(func, object, NULL, value);
 	}
 }
@@ -6155,42 +6173,46 @@ ZEND_METHOD(ReflectionProperty, setRawValue)
 
 	GET_REFLECTION_OBJECT_PTR(ref);
 
-	if (prop_get_flags(ref) & ZEND_ACC_STATIC) {
-		_DO_THROW("May not use setRawValue on static properties");
-		RETURN_THROWS();
-	}
-
 	if (zend_parse_parameters(ZEND_NUM_ARGS(), "oz", &object, &value) == FAILURE) {
 		RETURN_THROWS();
 	}
 
-	reflection_property_set_raw_value(ref, intern, Z_OBJ_P(object), value);
+	zend_property_info *prop = reflection_property_get_effective_prop(ref,
+			intern->ce, Z_OBJ_P(object));
+
+	if (UNEXPECTED(prop && (prop->flags & ZEND_ACC_STATIC))) {
+		_DO_THROW("May not use setRawValue on static properties");
+		RETURN_THROWS();
+	}
+
+	reflection_property_set_raw_value(prop, ref->unmangled_name, intern, Z_OBJ_P(object), value);
 }
 
-static zend_result reflection_property_check_lazy_compatible(reflection_object *intern,
-		property_reference *ref, zend_object *object, const char *method)
+static zend_result reflection_property_check_lazy_compatible(
+		zend_property_info *prop, zend_string *unmangled_name,
+		reflection_object *intern, zend_object *object, const char *method)
 {
-	if (!ref->prop) {
+	if (!prop) {
 		zend_throw_exception_ex(reflection_exception_ptr, 0,
 				"Can not use %s on dynamic property %s::$%s",
 				method, ZSTR_VAL(intern->ce->name),
-				ZSTR_VAL(ref->unmangled_name));
+				ZSTR_VAL(unmangled_name));
 		return FAILURE;
 	}
 
-	if (ref->prop->flags & ZEND_ACC_STATIC) {
+	if (prop->flags & ZEND_ACC_STATIC) {
 		zend_throw_exception_ex(reflection_exception_ptr, 0,
 				"Can not use %s on static property %s::$%s",
-				method, ZSTR_VAL(intern->ce->name),
-				ZSTR_VAL(ref->unmangled_name));
+				method, ZSTR_VAL(prop->ce->name),
+				ZSTR_VAL(unmangled_name));
 		return FAILURE;
 	}
 
-	if (ref->prop->flags & ZEND_ACC_VIRTUAL) {
+	if (prop->flags & ZEND_ACC_VIRTUAL) {
 		zend_throw_exception_ex(reflection_exception_ptr, 0,
 				"Can not use %s on virtual property %s::$%s",
-				method, ZSTR_VAL(intern->ce->name),
-				ZSTR_VAL(ref->unmangled_name));
+				method, ZSTR_VAL(prop->ce->name),
+				ZSTR_VAL(unmangled_name));
 		return FAILURE;
 	}
 
@@ -6198,12 +6220,12 @@ static zend_result reflection_property_check_lazy_compatible(reflection_object *
 		if (!zend_class_can_be_lazy(object->ce)) {
 			zend_throw_exception_ex(reflection_exception_ptr, 0,
 					"Can not use %s on internal class %s",
-					method, ZSTR_VAL(intern->ce->name));
+					method, ZSTR_VAL(object->ce->name));
 			return FAILURE;
 		}
 	}
 
-	ZEND_ASSERT(IS_VALID_PROPERTY_OFFSET(ref->prop->offset));
+	ZEND_ASSERT(IS_VALID_PROPERTY_OFFSET(prop->offset));
 
 	return SUCCESS;
 }
@@ -6223,23 +6245,27 @@ ZEND_METHOD(ReflectionProperty, setRawValueWithoutLazyInitialization)
 		Z_PARAM_ZVAL(value)
 	} ZEND_PARSE_PARAMETERS_END();
 
-	if (reflection_property_check_lazy_compatible(intern, ref, object,
-				"setRawValueWithoutLazyInitialization") == FAILURE) {
-		RETURN_THROWS();
-	}
-
 	while (zend_object_is_lazy_proxy(object)
 			&& zend_lazy_object_initialized(object)) {
 		object = zend_lazy_object_get_instance(object);
 	}
 
-	zval *var_ptr = OBJ_PROP(object, ref->prop->offset);
+	zend_property_info *prop = reflection_property_get_effective_prop(ref,
+			intern->ce, object);
+
+	if (reflection_property_check_lazy_compatible(prop, ref->unmangled_name,
+				intern, object, "setRawValueWithoutLazyInitialization") == FAILURE) {
+		RETURN_THROWS();
+	}
+
+	zval *var_ptr = OBJ_PROP(object, prop->offset);
 	bool prop_was_lazy = Z_PROP_FLAG_P(var_ptr) & IS_PROP_LAZY;
 
 	/* Do not trigger initialization */
 	Z_PROP_FLAG_P(var_ptr) &= ~IS_PROP_LAZY;
 
-	reflection_property_set_raw_value(ref, intern, object, value);
+	reflection_property_set_raw_value(prop, ref->unmangled_name, intern, object,
+			value);
 
 	/* Mark property as lazy again if an exception prevented update */
 	if (EG(exception) && prop_was_lazy && Z_TYPE_P(var_ptr) == IS_UNDEF
@@ -6271,7 +6297,8 @@ ZEND_METHOD(ReflectionProperty, skipLazyInitialization)
 		Z_PARAM_OBJ_OF_CLASS(object, intern->ce)
 	} ZEND_PARSE_PARAMETERS_END();
 
-	if (reflection_property_check_lazy_compatible(intern, ref, object,
+	if (reflection_property_check_lazy_compatible(ref->prop,
+				ref->unmangled_name, intern, object,
 				"skipLazyInitialization") == FAILURE) {
 		RETURN_THROWS();
 	}

ext/reflection/tests/property_hooks/gh17713.phpt

@@ -0,0 +1,169 @@
+--TEST--
+GH-17713: getRawValue(), setRawValue(), setRawValueWithoutLazyInitialization() may call hooks
+--FILE--
+<?php
+
+#[AllowDynamicProperties]
+class Base {
+    public $publicProp = 'Base::$publicProp';
+    private $privateProp = 'Base::$privateProp';
+    public $hookedProp {
+        get {
+            echo __METHOD__, "\n";
+            return $this->hookedProp;
+        }
+        set {
+            echo __METHOD__, "\n";
+            $this->hookedProp = $value;
+        }
+    }
+    public $virtualProp {
+        get {
+            echo __METHOD__, "\n";
+            return __METHOD__;
+        }
+        set {
+            echo __METHOD__, "\n";
+        }
+    }
+}
+
+class Test extends Base {
+    public $publicProp {
+        get {
+            echo __FUNCTION__, "\n";
+            return $this->publicProp;
+        }
+        set {
+            echo __FUNCTION__, "\n";
+            $this->publicProp = $value;
+        }
+    }
+    private $privateProp = 'Test::$privateProp' {
+        get {
+            echo __FUNCTION__, "\n";
+            return $this->privateProp;
+        }
+        set {
+            echo __FUNCTION__, "\n";
+            $this->privateProp = $value;
+        }
+    }
+    public $dynamicProp {
+        get {
+            echo __FUNCTION__, "\n";
+            return $this->dynamicProp;
+        }
+        set {
+            echo __FUNCTION__, "\n";
+            $this->dynamicProp = $value;
+        }
+    }
+    public $hookedProp {
+        get {
+            echo __METHOD__, "\n";
+            return $this->hookedProp;
+        }
+        set {
+            echo __METHOD__, "\n";
+            $this->hookedProp = $value;
+        }
+    }
+    public $virtualProp {
+        get {
+            echo __METHOD__, "\n";
+            return $this->virtualProp;
+        }
+        set {
+            echo __METHOD__, "\n";
+            $this->virtualProp = $value;
+        }
+    }
+    public static $changedProp = 'Test::$changedProp';
+}
+
+function test($scope, $class, $prop) {
+    printf(
+        "# Accessing %s->%s from scope %s\n",
+        $class,
+        $prop,
+        is_object($scope) ? get_class($scope) : $scope,
+    );
+
+    $propertyReflection = new ReflectionProperty($scope, $prop);
+    $object = new $class();
+    try {
+        $propertyReflection->setRawValue($object, 42);
+    } catch (Throwable $e) {
+        echo $e->getMessage(), "\n";
+    }
+    try {
+        var_dump($propertyReflection->getRawValue($object));
+    } catch (Throwable $e) {
+        echo $e->getMessage(), "\n";
+    }
+
+    try {
+        $propertyReflection->setRawValueWithoutLazyInitialization($object, 43);
+    } catch (Throwable $e) {
+        echo $e->getMessage(), "\n";
+    }
+    try {
+        var_dump($propertyReflection->getRawValue($object));
+    } catch (Throwable $e) {
+        echo $e->getMessage(), "\n";
+    }
+    if ($prop === 'privateProp') {
+        printf(
+            "# Accessing %s->%s again from scope %s\n",
+            $class,
+            $prop,
+            $class,
+        );
+        $propertyReflection = new ReflectionProperty($class, $prop);
+        var_dump($propertyReflection->getRawValue($object));
+    }
+}
+
+test(Base::class, Test::class, 'publicProp');
+test(Base::class, Test::class, 'privateProp');
+test(Base::class, Test::class, 'hookedProp');
+test(Base::class, Test::class, 'virtualProp');
+test(Base::class, Base::class, 'virtualProp');
+
+$scope = new Base();
+$scope->dynamicProp = 'Base::$dynamicProp';
+$scope->changedProp = 'Base::$dynamicProp';
+
+test($scope, Test::class, 'dynamicProp');
+test($scope, Test::class, 'changedProp');
+
+?>
+--EXPECT--
+# Accessing Test->publicProp from scope Base
+int(42)
+int(43)
+# Accessing Test->privateProp from scope Base
+int(42)
+int(43)
+# Accessing Test->privateProp again from scope Test
+string(18) "Test::$privateProp"
+# Accessing Test->hookedProp from scope Base
+int(42)
+int(43)
+# Accessing Test->virtualProp from scope Base
+int(42)
+int(43)
+# Accessing Base->virtualProp from scope Base
+Must not write to virtual property Base::$virtualProp
+Must not read from virtual property Base::$virtualProp
+Can not use setRawValueWithoutLazyInitialization on virtual property Base::$virtualProp
+Must not read from virtual property Base::$virtualProp
+# Accessing Test->dynamicProp from scope Base
+int(42)
+int(43)
+# Accessing Test->changedProp from scope Base
+May not use setRawValue on static properties
+May not use getRawValue on static properties
+Can not use setRawValueWithoutLazyInitialization on static property Test::$changedProp
+May not use getRawValue on static properties