Fixed bug #76509

In PHP static properties are shared between inheriting classes,
unless they are explicitly overwritten. However, because this
functionality was implemented using reference, it was possible
to break the implementation by reassigning the static property
reference.

This is fixed by switching the implementation from using references
to using INDIRECTs, which cannot be affected by userland code.

NEWS

@@ -7,6 +7,8 @@ PHP                                                                        NEWS
     (Nikita)
   . Fixed bug #76502 (Chain of mixed exceptions and errors does not serialize
     properly). (Nikita)
+  . Fixed bug #76509 (Inherited static properties can be desynchronized from
+    their parent by ref). (Nikita)
 
 - FPM:
   . Fixed bug #73342 (Vulnerability in php-fpm by changing stdin to

UPGRADING

@@ -44,6 +44,23 @@ Core:
     the following "FOO;" will cause a syntax error. This issue can always be
     resolved by choosing an ending label that does not occur within the contents
     of the string.
+  . In PHP, static properties are shared between inheriting classes, unless the
+    static property is explicitly overridden in a child class. However, due to
+    an implementation artifact it was possible to separate the static properties
+    by assigning a reference. This loophole has been fixed.
+
+        class Test {
+            public static $x = 0;
+        }
+        class Test2 extends Test { }
+
+        Test2::$x = &$x;
+        $x = 1;
+
+        var_dump(Test::$x, Test2::$x);
+        // Previously: int(0), int(1)
+        // Now:        int(1), int(1)
+
   . References returned by array and property accesses are now unwrapped as
     part of the access. This means that it is no longer possible to modify the
     reference between the access and the use of the accessed value:

Zend/zend_API.c

@@ -1164,19 +1164,10 @@ ZEND_API int zend_update_class_constants(zend_class_entry *class_type) /* {{{ */
 #endif
 			for (i = 0; i < class_type->default_static_members_count; i++) {
 				p = &class_type->default_static_members_table[i];
-				if (Z_ISREF_P(p)) {
-					if (class_type->parent &&
-						i < class_type->parent->default_static_members_count &&
-						p == &class_type->parent->default_static_members_table[i] &&
-						Z_TYPE(CE_STATIC_MEMBERS(class_type->parent)[i]) != IS_UNDEF
-					) {
-						zval *q = &CE_STATIC_MEMBERS(class_type->parent)[i];
-
-						ZVAL_NEW_REF(q, q);
-						ZVAL_COPY(&CE_STATIC_MEMBERS(class_type)[i], q);
-					} else {
-						ZVAL_COPY_OR_DUP(&CE_STATIC_MEMBERS(class_type)[i], Z_REFVAL_P(p));
-					}
+				if (Z_TYPE_P(p) == IS_INDIRECT) {
+					zval *q = &CE_STATIC_MEMBERS(class_type->parent)[i];
+					ZVAL_DEINDIRECT(q);
+					ZVAL_INDIRECT(&CE_STATIC_MEMBERS(class_type)[i], q);
 				} else {
 					ZVAL_COPY_OR_DUP(&CE_STATIC_MEMBERS(class_type)[i], p);
 				}

Zend/zend_builtin_functions.c

@@ -1071,6 +1071,7 @@ static void add_class_vars(zend_class_entry *scope, zend_class_entry *ce, int st
 		prop = NULL;
 		if (statics && (prop_info->flags & ZEND_ACC_STATIC) != 0) {
 			prop = &ce->default_static_members_table[prop_info->offset];
+			ZVAL_DEINDIRECT(prop);
 		} else if (!statics && (prop_info->flags & ZEND_ACC_STATIC) == 0) {
 			prop = &ce->default_properties_table[OBJ_PROP_TO_NUM(prop_info->offset)];
 		}

Zend/zend_inheritance.c

@@ -890,25 +890,23 @@ ZEND_API void zend_do_inheritance(zend_class_entry *ce, zend_class_entry *parent
 			do {
 				dst--;
 				src--;
-				if (Z_ISREF_P(src)) {
-					Z_ADDREF_P(src);
+				if (Z_TYPE_P(src) == IS_INDIRECT) {
+					ZVAL_INDIRECT(dst, Z_INDIRECT_P(src));
 				} else {
-					ZVAL_MAKE_REF_EX(src, 2);
+					ZVAL_INDIRECT(dst, src);
 				}
-				ZVAL_REF(dst, Z_REF_P(src));
 			} while (dst != end);
 		} else if (ce->type == ZEND_USER_CLASS) {
 			src = parent_ce->default_static_members_table + parent_ce->default_static_members_count;
 			do {
 				dst--;
 				src--;
-				if (Z_ISREF_P(src)) {
-					Z_ADDREF_P(src);
+				if (Z_TYPE_P(src) == IS_INDIRECT) {
+					ZVAL_INDIRECT(dst, Z_INDIRECT_P(src));
 				} else {
-					ZVAL_MAKE_REF_EX(src, 2);
+					ZVAL_INDIRECT(dst, src);
 				}
-				ZVAL_REF(dst, Z_REF_P(src));
-				if (Z_TYPE_P(Z_REFVAL_P(dst)) == IS_CONSTANT_AST) {
+				if (Z_TYPE_P(Z_INDIRECT_P(dst)) == IS_CONSTANT_AST) {
 					ce->ce_flags &= ~ZEND_ACC_CONSTANTS_UPDATED;
 				}
 			} while (dst != end);
@@ -917,11 +915,11 @@ ZEND_API void zend_do_inheritance(zend_class_entry *ce, zend_class_entry *parent
 			do {
 				dst--;
 				src--;
-				if (!Z_ISREF_P(src)) {
-					ZVAL_NEW_PERSISTENT_REF(src, src);
+				if (Z_TYPE_P(src) == IS_INDIRECT) {
+					ZVAL_INDIRECT(dst, Z_INDIRECT_P(src));
+				} else {
+					ZVAL_INDIRECT(dst, src);
 				}
-				ZVAL_COPY_VALUE(dst, src);
-				Z_ADDREF_P(dst);
 			} while (dst != end);
 		}
 		ce->default_static_members_count += parent_ce->default_static_members_count;
@@ -1605,8 +1603,8 @@ static void zend_do_traits_property_binding(zend_class_entry *ce) /* {{{ */
 						if (flags & ZEND_ACC_STATIC) {
 							op1 = &ce->default_static_members_table[coliding_prop->offset];
 							op2 = &ce->traits[i]->default_static_members_table[property_info->offset];
-							ZVAL_DEREF(op1);
-							ZVAL_DEREF(op2);
+							ZVAL_DEINDIRECT(op1);
+							ZVAL_DEINDIRECT(op2);
 						} else {
 							op1 = &ce->default_properties_table[OBJ_PROP_TO_NUM(coliding_prop->offset)];
 							op2 = &ce->traits[i]->default_properties_table[OBJ_PROP_TO_NUM(property_info->offset)];
@@ -1651,6 +1649,7 @@ static void zend_do_traits_property_binding(zend_class_entry *ce) /* {{{ */
 			/* property not found, so lets add it */
 			if (flags & ZEND_ACC_STATIC) {
 				prop_value = &ce->traits[i]->default_static_members_table[property_info->offset];
+				ZEND_ASSERT(Z_TYPE_P(prop_value) != IS_INDIRECT);
 			} else {
 				prop_value = &ce->traits[i]->default_properties_table[OBJ_PROP_TO_NUM(property_info->offset)];
 			}

Zend/zend_object_handlers.c

@@ -1445,17 +1445,18 @@ ZEND_API zval *zend_std_get_static_property(zend_class_entry *ce, zend_string *p
 			return NULL;
 		}
 	}
-	ret = CE_STATIC_MEMBERS(ce) + property_info->offset;
 
 	/* check if static properties were destoyed */
 	if (UNEXPECTED(CE_STATIC_MEMBERS(ce) == NULL)) {
 undeclared_property:
 		if (!silent) {
 			zend_throw_error(NULL, "Access to undeclared static property: %s::$%s", ZSTR_VAL(ce->name), ZSTR_VAL(property_name));
 		}
-		ret = NULL;
+		return NULL;
 	}
 
+	ret = CE_STATIC_MEMBERS(ce) + property_info->offset;
+	ZVAL_DEINDIRECT(ret);
 	return ret;
 }
 /* }}} */

Zend/zend_types.h

@@ -1101,6 +1101,12 @@ static zend_always_inline uint32_t zval_delref_p(zval* pz) {
 		}												\
 	} while (0)
 
+#define ZVAL_DEINDIRECT(z) do {							\
+		if (Z_TYPE_P(z) == IS_INDIRECT) {				\
+			(z) = Z_INDIRECT_P(z);						\
+		}												\
+	} while (0)
+
 #define ZVAL_OPT_DEREF(z) do {							\
 		if (UNEXPECTED(Z_OPT_ISREF_P(z))) {				\
 			(z) = Z_REFVAL_P(z);						\

ext/opcache/zend_accelerator_util_funcs.c

@@ -346,6 +346,10 @@ static void zend_class_copy_ctor(zend_class_entry **pce)
 	*pce = ce = ARENA_REALLOC(old_ce);
 	ce->refcount = 1;
 
+	if (ce->parent) {
+		ce->parent = ARENA_REALLOC(ce->parent);
+	}
+
 	if (old_ce->default_properties_table) {
 		ce->default_properties_table = emalloc(sizeof(zval) * old_ce->default_properties_count);
 		src = old_ce->default_properties_table;
@@ -361,13 +365,29 @@ static void zend_class_copy_ctor(zend_class_entry **pce)
 
 	/* static members */
 	if (old_ce->default_static_members_table) {
+		int i, end;
+		zend_class_entry *parent = ce->parent;
+
 		ce->default_static_members_table = emalloc(sizeof(zval) * old_ce->default_static_members_count);
-		src = old_ce->default_static_members_table;
-		end = src + old_ce->default_static_members_count;
-		dst = ce->default_static_members_table;
-		for (; src != end; src++, dst++) {
-			ZVAL_COPY_VALUE(dst, src);
-			zend_clone_zval(dst);
+		i = ce->default_static_members_count;
+
+		/* Copy static properties in this class */
+		end = parent ? parent->default_static_members_count : 0;
+		for (; i >= end; i--) {
+			zval *p = &ce->default_static_members_table[i];
+			ZVAL_COPY_VALUE(p, &old_ce->default_static_members_table[i]);
+			zend_clone_zval(p);
+		}
+
+		/* Create indirections to static properties from parent classes */
+		while (parent && parent->default_static_members_table) {
+			end = parent->parent ? parent->parent->default_static_members_count : 0;
+			for (; i >= end; i--) {
+				zval *p = &ce->default_static_members_table[i];
+				ZVAL_INDIRECT(p, &parent->default_static_members_table[i]);
+			}
+
+			parent = parent->parent;
 		}
 	}
 	ce->static_members_table = ce->default_static_members_table;
@@ -386,10 +406,6 @@ static void zend_class_copy_ctor(zend_class_entry **pce)
 		ce->interfaces = NULL;
 	}
 
-	if (ce->parent) {
-		ce->parent = ARENA_REALLOC(ce->parent);
-	}
-
 	zend_update_inherited_handler(constructor);
 	zend_update_inherited_handler(destructor);
 	zend_update_inherited_handler(clone);

ext/opcache/zend_file_cache.c

@@ -612,12 +612,16 @@ static void zend_file_cache_serialize_class(zval                     *zv,
 		}
 	}
 	if (ce->default_static_members_table) {
-		zval *p, *end;
+		zval *table, *p, *end;
 
 		SERIALIZE_PTR(ce->default_static_members_table);
-		p = ce->default_static_members_table;
-		UNSERIALIZE_PTR(p);
-		end = p + ce->default_static_members_count;
+		table = ce->default_static_members_table;
+		UNSERIALIZE_PTR(table);
+
+		/* Serialize only static properties in this class.
+		 * Static properties from parent classes will be handled in class_copy_ctor */
+		p = table + (ce->parent ? ce->parent->default_static_members_count : 0);
+		end = table + ce->default_static_members_count;
 		while (p < end) {
 			zend_file_cache_serialize_zval(p, script, info, buf);
 			p++;
@@ -1225,6 +1229,7 @@ static void zend_file_cache_unserialize_class(zval                    *zv,
 	ce = Z_PTR_P(zv);
 
 	UNSERIALIZE_STR(ce->name);
+	UNSERIALIZE_PTR(ce->parent);
 	zend_file_cache_unserialize_hash(&ce->function_table,
 			script, buf, zend_file_cache_unserialize_func, ZEND_FUNCTION_DTOR);
 	if (ce->default_properties_table) {
@@ -1239,11 +1244,14 @@ static void zend_file_cache_unserialize_class(zval                    *zv,
 		}
 	}
 	if (ce->default_static_members_table) {
-		zval *p, *end;
+		zval *table, *p, *end;
 
+		/* Unserialize only static properties in this class.
+		 * Static properties from parent classes will be handled in class_copy_ctor */
 		UNSERIALIZE_PTR(ce->default_static_members_table);
-		p = ce->default_static_members_table;
-		end = p + ce->default_static_members_count;
+		table = ce->default_static_members_table;
+		p = table + (ce->parent ? ce->parent->default_static_members_count : 0);
+		end = table + ce->default_static_members_count;
 		while (p < end) {
 			zend_file_cache_unserialize_zval(p, script, buf);
 			p++;
@@ -1326,7 +1334,6 @@ static void zend_file_cache_unserialize_class(zval                    *zv,
 		}
 	}
 
-	UNSERIALIZE_PTR(ce->parent);
 	UNSERIALIZE_PTR(ce->constructor);
 	UNSERIALIZE_PTR(ce->destructor);
 	UNSERIALIZE_PTR(ce->clone);

ext/opcache/zend_persist.c

@@ -720,10 +720,13 @@ static void zend_persist_class_entry(zval *zv)
 			}
 		}
 		if (ce->default_static_members_table) {
-		    int i;
-
+			int i;
 			zend_accel_store(ce->default_static_members_table, sizeof(zval) * ce->default_static_members_count);
-			for (i = 0; i < ce->default_static_members_count; i++) {
+
+			/* Persist only static properties in this class.
+			 * Static properties from parent classes will be handled in class_copy_ctor */
+			i = ce->parent ? ce->parent->default_static_members_count : 0;
+			for (; i < ce->default_static_members_count; i++) {
 				zend_persist_zval(&ce->default_static_members_table[i]);
 			}
 		}

ext/opcache/zend_persist_calc.c

@@ -326,7 +326,9 @@ static void zend_persist_class_entry_calc(zval *zv)
 
 			ADD_SIZE(sizeof(zval) * ce->default_static_members_count);
 			for (i = 0; i < ce->default_static_members_count; i++) {
-				zend_persist_zval_calc(&ce->default_static_members_table[i]);
+				if (Z_TYPE(ce->default_static_members_table[i]) != IS_INDIRECT) {
+					zend_persist_zval_calc(&ce->default_static_members_table[i]);
+				}
 			}
 		}
 		zend_hash_persist_calc(&ce->constants_table, zend_persist_class_constant_calc);

ext/reflection/php_reflection.c

@@ -3795,6 +3795,7 @@ static void add_class_vars(zend_class_entry *ce, int statics, zval *return_value
 		prop = NULL;
 		if (statics && (prop_info->flags & ZEND_ACC_STATIC) != 0) {
 			prop = &ce->default_static_members_table[prop_info->offset];
+			ZVAL_DEINDIRECT(prop);
 		} else if (!statics && (prop_info->flags & ZEND_ACC_STATIC) == 0) {
 			prop = &ce->default_properties_table[OBJ_PROP_TO_NUM(prop_info->offset)];
 		}
@@ -5503,6 +5504,7 @@ ZEND_METHOD(reflection_property, getValue)
 			return;
 		}
 		member_p = &CE_STATIC_MEMBERS(intern->ce)[ref->prop.offset];
+		ZVAL_DEINDIRECT(member_p);
 		ZVAL_DEREF(member_p);
 		ZVAL_COPY(return_value, member_p);
 	} else {
@@ -5570,6 +5572,7 @@ ZEND_METHOD(reflection_property, setValue)
 			return;
 		}
 		variable_ptr = &CE_STATIC_MEMBERS(intern->ce)[ref->prop.offset];
+		ZVAL_DEINDIRECT(variable_ptr);
 		if (variable_ptr != value) {
 			zval garbage;
 

tests/classes/static_properties_004.phpt

@@ -1,5 +1,5 @@
 --TEST--
-Inherited static properties can be separated from their reference set. 
+Inherited static properties cannot be separated from their reference set. 
 --FILE--
 <?php
 class C { public static $p = 'original'; }
@@ -13,7 +13,7 @@ echo "\nChanging one changes all the others:\n";
 D::$p = 'changed.all';
 var_dump(C::$p, D::$p, E::$p);
 
-echo "\nBut because this is implemented using PHP references, the reference set can easily be split:\n";
+echo "\nReferences cannot be used to split the properties:\n";
 $ref = 'changed.one';
 D::$p =& $ref;
 var_dump(C::$p, D::$p, E::$p);
@@ -30,8 +30,8 @@ string(11) "changed.all"
 string(11) "changed.all"
 string(11) "changed.all"
 
-But because this is implemented using PHP references, the reference set can easily be split:
-string(11) "changed.all"
+References cannot be used to split the properties:
+string(11) "changed.one"
+string(11) "changed.one"
 string(11) "changed.one"
-string(11) "changed.all"
 ==Done==