Disallow access of backing value outside of hook

php · Apr 29, 2024 · 01ca5f7 · 01ca5f7
1 parent 6029234
commit 01ca5f7
Show file tree

Hide file tree

Showing 3 changed files with 106 additions and 0 deletions.
diff --git a/Zend/tests/property_hooks/backed_delegated_read_wirte.phpt b/Zend/tests/property_hooks/backed_delegated_read_wirte.phpt
@@ -0,0 +1,57 @@
+--TEST--
+Attempted read/write of backing value in a delegated method throws
+--FILE--
+<?php
+
+class Test {
+    public $prop = 42 {
+        get => $this->getProp($this->prop);
+        set {
+            $this->setProp($this->prop, $value);
+        }
+    }
+
+    private function getProp($prop) {
+        return $this->prop;
+    }
+
+    private function setProp($prop, $value) {
+        $this->prop = $value;
+    }
+
+    public $prop2 = 42 {
+        get => $this->prop2;
+        set => $value;
+    }
+}
+
+class Child extends Test {
+    public $prop2 = 42 {
+        get => parent::$prop2::get();
+        set { parent::$prop2::set($value); }
+    }
+}
+
+$test = new Test;
+
+try {
+    $test->prop = 0;
+} catch (Error $e) {
+    echo $e->getMessage(), "\n";
+}
+
+try {
+    var_dump($test->prop);
+} catch (Error $e) {
+    echo $e->getMessage(), "\n";
+}
+
+$child = new Child();
+$child->prop2 = 43;
+var_dump($child->prop2);
+
+?>
+--EXPECT--
+Must not write to property Test::$prop recursively
+Must not read from property Test::$prop recursively
+int(43)
diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c
@@ -1315,6 +1315,8 @@ static void inherit_property_hook(
 		return;
 	}
 
+	child->common.prototype = parent->common.prototype ? parent->common.prototype : parent;
+
 	uint32_t parent_flags = parent->common.fn_flags;
 	if (parent_flags & ZEND_ACC_PRIVATE) {
 		child->common.fn_flags |= ZEND_ACC_CHANGED;

diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c
@@ -623,6 +623,44 @@ ZEND_COLD static void zend_typed_property_uninitialized_access(const zend_proper
 		ZSTR_VAL(name));
 }
 
+static ZEND_FUNCTION(zend_parent_hook_get_trampoline);
+static ZEND_FUNCTION(zend_parent_hook_set_trampoline);
+
+static bool is_in_hook(const zend_property_info *prop_info)
+{
+	zend_execute_data *execute_data = EG(current_execute_data);
+	if (!execute_data || !EX(func)) {
+		return false;
+	}
+
+	zend_function *func = EX(func);
+	zend_function **hooks = prop_info->hooks;
+	ZEND_ASSERT(hooks);
+
+	/* Fast path: Check if we're directly in the properties hook. */
+	if (func == hooks[ZEND_PROPERTY_HOOK_GET]
+	 || func == hooks[ZEND_PROPERTY_HOOK_SET]) {
+		return true;
+	}
+
+	/* Check if we're in a parent hook. */
+	zend_function *prototype = func->common.prototype ? func->common.prototype : func;
+	if ((hooks[ZEND_PROPERTY_HOOK_GET] && prototype == hooks[ZEND_PROPERTY_HOOK_GET]->common.prototype)
+	 || (hooks[ZEND_PROPERTY_HOOK_SET] && prototype == hooks[ZEND_PROPERTY_HOOK_SET]->common.prototype)) {
+		return true;
+	}
+
+	/* Check if we're in the trampoline helper. */
+	if (EX(func) == &EG(trampoline)
+	 && !ZEND_USER_CODE(EX(func)->type)
+	 && (EX(func)->internal_function.handler == ZEND_FN(zend_parent_hook_get_trampoline)
+	  || EX(func)->internal_function.handler == ZEND_FN(zend_parent_hook_set_trampoline))) {
+		return true;
+	}
+
+	return false;
+}
+
 ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int type, void **cache_slot, zval *rv) /* {{{ */
 {
 	zval *retval;
@@ -738,6 +776,9 @@ ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int
 				zend_throw_error(NULL, "Must not read from virtual property %s::$%s",
 					ZSTR_VAL(zobj->ce->name), ZSTR_VAL(name));
 				return &EG(uninitialized_zval);
+			} else if (UNEXPECTED(!is_in_hook(prop_info))) {
+				zend_throw_error(NULL, "Must not read from property %s::$%s recursively",
+					ZSTR_VAL(zobj->ce->name), ZSTR_VAL(name));
 			}
 			property_offset = prop_info->offset;
 			if (!ZEND_TYPE_IS_SET(prop_info->type)) {
@@ -1000,6 +1041,9 @@ found:;
 					 ZSTR_VAL(zobj->ce->name), ZSTR_VAL(name));
 				variable_ptr = &EG(error_zval);
 				goto exit;
+			} else if (UNEXPECTED(!is_in_hook(prop_info))) {
+				zend_throw_error(NULL, "Must not write to property %s::$%s recursively",
+					ZSTR_VAL(zobj->ce->name), ZSTR_VAL(name));
 			}
 			property_offset = prop_info->offset;
 			if (!ZEND_TYPE_IS_SET(prop_info->type)) {
@@ -2115,6 +2159,9 @@ ZEND_API int zend_std_has_property(zend_object *zobj, zend_string *name, int has
 				zend_throw_error(NULL, "Must not read from virtual property %s::$%s",
 					ZSTR_VAL(zobj->ce->name), ZSTR_VAL(name));
 				return 0;
+			} else if (UNEXPECTED(!is_in_hook(prop_info))) {
+				zend_throw_error(NULL, "Must not read from property %s::$%s recursively",
+					ZSTR_VAL(zobj->ce->name), ZSTR_VAL(name));
 			}
 			property_offset = prop_info->offset;
 			if (!ZEND_TYPE_IS_SET(prop_info->type)) {