Skip to content

Commit 2543e61

Browse files
nikicnikic
committed
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.
1 parent 102bcb5 commit 2543e61

13 files changed

+105
-57
lines changed

‎NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ PHP NEWS
77
(Nikita)
88
. Fixed bug #76502 (Chain of mixed exceptions and errors does not serialize
99
properly). (Nikita)
10+
. Fixed bug #76509 (Inherited static properties can be desynchronized from
11+
their parent by ref). (Nikita)
1012

1113
- FPM:
1214
. Fixed bug #73342 (Vulnerability in php-fpm by changing stdin to

‎UPGRADING

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,23 @@ Core:
4444
the following "FOO;" will cause a syntax error. This issue can always be
4545
resolved by choosing an ending label that does not occur within the contents
4646
of the string.
47+
. In PHP, static properties are shared between inheriting classes, unless the
48+
static property is explicitly overridden in a child class. However, due to
49+
an implementation artifact it was possible to separate the static properties
50+
by assigning a reference. This loophole has been fixed.
51+
52+
class Test {
53+
public static $x = 0;
54+
}
55+
class Test2 extends Test { }
56+
57+
Test2::$x = &$x;
58+
$x = 1;
59+
60+
var_dump(Test::$x, Test2::$x);
61+
// Previously: int(0), int(1)
62+
// Now: int(1), int(1)
63+
4764
. References returned by array and property accesses are now unwrapped as
4865
part of the access. This means that it is no longer possible to modify the
4966
reference between the access and the use of the accessed value:

‎Zend/zend_API.c

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1164,19 +1164,10 @@ ZEND_API int zend_update_class_constants(zend_class_entry *class_type) /* {{{ */
11641164
#endif
11651165
for (i = 0; i < class_type->default_static_members_count; i++) {
11661166
p = &class_type->default_static_members_table[i];
1167-
if (Z_ISREF_P(p)) {
1168-
if (class_type->parent &&
1169-
i < class_type->parent->default_static_members_count &&
1170-
p == &class_type->parent->default_static_members_table[i] &&
1171-
Z_TYPE(CE_STATIC_MEMBERS(class_type->parent)[i]) != IS_UNDEF
1172-
) {
1173-
zval *q = &CE_STATIC_MEMBERS(class_type->parent)[i];
1174-
1175-
ZVAL_NEW_REF(q, q);
1176-
ZVAL_COPY(&CE_STATIC_MEMBERS(class_type)[i], q);
1177-
} else {
1178-
ZVAL_COPY_OR_DUP(&CE_STATIC_MEMBERS(class_type)[i], Z_REFVAL_P(p));
1179-
}
1167+
if (Z_TYPE_P(p) == IS_INDIRECT) {
1168+
zval *q = &CE_STATIC_MEMBERS(class_type->parent)[i];
1169+
ZVAL_DEINDIRECT(q);
1170+
ZVAL_INDIRECT(&CE_STATIC_MEMBERS(class_type)[i], q);
11801171
} else {
11811172
ZVAL_COPY_OR_DUP(&CE_STATIC_MEMBERS(class_type)[i], p);
11821173
}

‎Zend/zend_builtin_functions.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1071,6 +1071,7 @@ static void add_class_vars(zend_class_entry *scope, zend_class_entry *ce, int st
10711071
prop = NULL;
10721072
if (statics && (prop_info->flags & ZEND_ACC_STATIC) != 0) {
10731073
prop = &ce->default_static_members_table[prop_info->offset];
1074+
ZVAL_DEINDIRECT(prop);
10741075
} else if (!statics && (prop_info->flags & ZEND_ACC_STATIC) == 0) {
10751076
prop = &ce->default_properties_table[OBJ_PROP_TO_NUM(prop_info->offset)];
10761077
}

‎Zend/zend_inheritance.c

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -890,25 +890,23 @@ ZEND_API void zend_do_inheritance(zend_class_entry *ce, zend_class_entry *parent
890890
do {
891891
dst--;
892892
src--;
893-
if (Z_ISREF_P(src)) {
894-
Z_ADDREF_P(src);
893+
if (Z_TYPE_P(src) == IS_INDIRECT) {
894+
ZVAL_INDIRECT(dst, Z_INDIRECT_P(src));
895895
} else {
896-
ZVAL_MAKE_REF_EX(src, 2);
896+
ZVAL_INDIRECT(dst, src);
897897
}
898-
ZVAL_REF(dst, Z_REF_P(src));
899898
} while (dst != end);
900899
} else if (ce->type == ZEND_USER_CLASS) {
901900
src = parent_ce->default_static_members_table + parent_ce->default_static_members_count;
902901
do {
903902
dst--;
904903
src--;
905-
if (Z_ISREF_P(src)) {
906-
Z_ADDREF_P(src);
904+
if (Z_TYPE_P(src) == IS_INDIRECT) {
905+
ZVAL_INDIRECT(dst, Z_INDIRECT_P(src));
907906
} else {
908-
ZVAL_MAKE_REF_EX(src, 2);
907+
ZVAL_INDIRECT(dst, src);
909908
}
910-
ZVAL_REF(dst, Z_REF_P(src));
911-
if (Z_TYPE_P(Z_REFVAL_P(dst)) == IS_CONSTANT_AST) {
909+
if (Z_TYPE_P(Z_INDIRECT_P(dst)) == IS_CONSTANT_AST) {
912910
ce->ce_flags &= ~ZEND_ACC_CONSTANTS_UPDATED;
913911
}
914912
} while (dst != end);
@@ -917,11 +915,11 @@ ZEND_API void zend_do_inheritance(zend_class_entry *ce, zend_class_entry *parent
917915
do {
918916
dst--;
919917
src--;
920-
if (!Z_ISREF_P(src)) {
921-
ZVAL_NEW_PERSISTENT_REF(src, src);
918+
if (Z_TYPE_P(src) == IS_INDIRECT) {
919+
ZVAL_INDIRECT(dst, Z_INDIRECT_P(src));
920+
} else {
921+
ZVAL_INDIRECT(dst, src);
922922
}
923-
ZVAL_COPY_VALUE(dst, src);
924-
Z_ADDREF_P(dst);
925923
} while (dst != end);
926924
}
927925
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) /* {{{ */
16051603
if (flags & ZEND_ACC_STATIC) {
16061604
op1 = &ce->default_static_members_table[coliding_prop->offset];
16071605
op2 = &ce->traits[i]->default_static_members_table[property_info->offset];
1608-
ZVAL_DEREF(op1);
1609-
ZVAL_DEREF(op2);
1606+
ZVAL_DEINDIRECT(op1);
1607+
ZVAL_DEINDIRECT(op2);
16101608
} else {
16111609
op1 = &ce->default_properties_table[OBJ_PROP_TO_NUM(coliding_prop->offset)];
16121610
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) /* {{{ */
16511649
/* property not found, so lets add it */
16521650
if (flags & ZEND_ACC_STATIC) {
16531651
prop_value = &ce->traits[i]->default_static_members_table[property_info->offset];
1652+
ZEND_ASSERT(Z_TYPE_P(prop_value) != IS_INDIRECT);
16541653
} else {
16551654
prop_value = &ce->traits[i]->default_properties_table[OBJ_PROP_TO_NUM(property_info->offset)];
16561655
}

‎Zend/zend_object_handlers.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1445,17 +1445,18 @@ ZEND_API zval *zend_std_get_static_property(zend_class_entry *ce, zend_string *p
14451445
return NULL;
14461446
}
14471447
}
1448-
ret = CE_STATIC_MEMBERS(ce) + property_info->offset;
14491448

14501449
/* check if static properties were destoyed */
14511450
if (UNEXPECTED(CE_STATIC_MEMBERS(ce) == NULL)) {
14521451
undeclared_property:
14531452
if (!silent) {
14541453
zend_throw_error(NULL, "Access to undeclared static property: %s::$%s", ZSTR_VAL(ce->name), ZSTR_VAL(property_name));
14551454
}
1456-
ret = NULL;
1455+
return NULL;
14571456
}
14581457

1458+
ret = CE_STATIC_MEMBERS(ce) + property_info->offset;
1459+
ZVAL_DEINDIRECT(ret);
14591460
return ret;
14601461
}
14611462
/* }}} */

‎Zend/zend_types.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1101,6 +1101,12 @@ static zend_always_inline uint32_t zval_delref_p(zval* pz) {
11011101
} \
11021102
} while (0)
11031103

1104+
#define ZVAL_DEINDIRECT(z) do { \
1105+
if (Z_TYPE_P(z) == IS_INDIRECT) { \
1106+
(z) = Z_INDIRECT_P(z); \
1107+
} \
1108+
} while (0)
1109+
11041110
#define ZVAL_OPT_DEREF(z) do { \
11051111
if (UNEXPECTED(Z_OPT_ISREF_P(z))) { \
11061112
(z) = Z_REFVAL_P(z); \

‎ext/opcache/zend_accelerator_util_funcs.c

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,10 @@ static void zend_class_copy_ctor(zend_class_entry **pce)
346346
*pce = ce = ARENA_REALLOC(old_ce);
347347
ce->refcount = 1;
348348

349+
if (ce->parent) {
350+
ce->parent = ARENA_REALLOC(ce->parent);
351+
}
352+
349353
if (old_ce->default_properties_table) {
350354
ce->default_properties_table = emalloc(sizeof(zval) * old_ce->default_properties_count);
351355
src = old_ce->default_properties_table;
@@ -361,13 +365,29 @@ static void zend_class_copy_ctor(zend_class_entry **pce)
361365

362366
/* static members */
363367
if (old_ce->default_static_members_table) {
368+
int i, end;
369+
zend_class_entry *parent = ce->parent;
370+
364371
ce->default_static_members_table = emalloc(sizeof(zval) * old_ce->default_static_members_count);
365-
src = old_ce->default_static_members_table;
366-
end = src + old_ce->default_static_members_count;
367-
dst = ce->default_static_members_table;
368-
for (; src != end; src++, dst++) {
369-
ZVAL_COPY_VALUE(dst, src);
370-
zend_clone_zval(dst);
372+
i = ce->default_static_members_count;
373+
374+
/* Copy static properties in this class */
375+
end = parent ? parent->default_static_members_count : 0;
376+
for (; i >= end; i--) {
377+
zval *p = &ce->default_static_members_table[i];
378+
ZVAL_COPY_VALUE(p, &old_ce->default_static_members_table[i]);
379+
zend_clone_zval(p);
380+
}
381+
382+
/* Create indirections to static properties from parent classes */
383+
while (parent && parent->default_static_members_table) {
384+
end = parent->parent ? parent->parent->default_static_members_count : 0;
385+
for (; i >= end; i--) {
386+
zval *p = &ce->default_static_members_table[i];
387+
ZVAL_INDIRECT(p, &parent->default_static_members_table[i]);
388+
}
389+
390+
parent = parent->parent;
371391
}
372392
}
373393
ce->static_members_table = ce->default_static_members_table;
@@ -386,10 +406,6 @@ static void zend_class_copy_ctor(zend_class_entry **pce)
386406
ce->interfaces = NULL;
387407
}
388408

389-
if (ce->parent) {
390-
ce->parent = ARENA_REALLOC(ce->parent);
391-
}
392-
393409
zend_update_inherited_handler(constructor);
394410
zend_update_inherited_handler(destructor);
395411
zend_update_inherited_handler(clone);

‎ext/opcache/zend_file_cache.c

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -612,12 +612,16 @@ static void zend_file_cache_serialize_class(zval *zv,
612612
}
613613
}
614614
if (ce->default_static_members_table) {
615-
zval *p, *end;
615+
zval *table, *p, *end;
616616

617617
SERIALIZE_PTR(ce->default_static_members_table);
618-
p = ce->default_static_members_table;
619-
UNSERIALIZE_PTR(p);
620-
end = p + ce->default_static_members_count;
618+
table = ce->default_static_members_table;
619+
UNSERIALIZE_PTR(table);
620+
621+
/* Serialize only static properties in this class.
622+
* Static properties from parent classes will be handled in class_copy_ctor */
623+
p = table + (ce->parent ? ce->parent->default_static_members_count : 0);
624+
end = table + ce->default_static_members_count;
621625
while (p < end) {
622626
zend_file_cache_serialize_zval(p, script, info, buf);
623627
p++;
@@ -1225,6 +1229,7 @@ static void zend_file_cache_unserialize_class(zval *zv,
12251229
ce = Z_PTR_P(zv);
12261230

12271231
UNSERIALIZE_STR(ce->name);
1232+
UNSERIALIZE_PTR(ce->parent);
12281233
zend_file_cache_unserialize_hash(&ce->function_table,
12291234
script, buf, zend_file_cache_unserialize_func, ZEND_FUNCTION_DTOR);
12301235
if (ce->default_properties_table) {
@@ -1239,11 +1244,14 @@ static void zend_file_cache_unserialize_class(zval *zv,
12391244
}
12401245
}
12411246
if (ce->default_static_members_table) {
1242-
zval *p, *end;
1247+
zval *table, *p, *end;
12431248

1249+
/* Unserialize only static properties in this class.
1250+
* Static properties from parent classes will be handled in class_copy_ctor */
12441251
UNSERIALIZE_PTR(ce->default_static_members_table);
1245-
p = ce->default_static_members_table;
1246-
end = p + ce->default_static_members_count;
1252+
table = ce->default_static_members_table;
1253+
p = table + (ce->parent ? ce->parent->default_static_members_count : 0);
1254+
end = table + ce->default_static_members_count;
12471255
while (p < end) {
12481256
zend_file_cache_unserialize_zval(p, script, buf);
12491257
p++;
@@ -1326,7 +1334,6 @@ static void zend_file_cache_unserialize_class(zval *zv,
13261334
}
13271335
}
13281336

1329-
UNSERIALIZE_PTR(ce->parent);
13301337
UNSERIALIZE_PTR(ce->constructor);
13311338
UNSERIALIZE_PTR(ce->destructor);
13321339
UNSERIALIZE_PTR(ce->clone);

‎ext/opcache/zend_persist.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -720,10 +720,13 @@ static void zend_persist_class_entry(zval *zv)
720720
}
721721
}
722722
if (ce->default_static_members_table) {
723-
int i;
724-
723+
int i;
725724
zend_accel_store(ce->default_static_members_table, sizeof(zval) * ce->default_static_members_count);
726-
for (i = 0; i < ce->default_static_members_count; i++) {
725+
726+
/* Persist only static properties in this class.
727+
* Static properties from parent classes will be handled in class_copy_ctor */
728+
i = ce->parent ? ce->parent->default_static_members_count : 0;
729+
for (; i < ce->default_static_members_count; i++) {
727730
zend_persist_zval(&ce->default_static_members_table[i]);
728731
}
729732
}

‎ext/opcache/zend_persist_calc.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,9 @@ static void zend_persist_class_entry_calc(zval *zv)
326326

327327
ADD_SIZE(sizeof(zval) * ce->default_static_members_count);
328328
for (i = 0; i < ce->default_static_members_count; i++) {
329-
zend_persist_zval_calc(&ce->default_static_members_table[i]);
329+
if (Z_TYPE(ce->default_static_members_table[i]) != IS_INDIRECT) {
330+
zend_persist_zval_calc(&ce->default_static_members_table[i]);
331+
}
330332
}
331333
}
332334
zend_hash_persist_calc(&ce->constants_table, zend_persist_class_constant_calc);

‎ext/reflection/php_reflection.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3795,6 +3795,7 @@ static void add_class_vars(zend_class_entry *ce, int statics, zval *return_value
37953795
prop = NULL;
37963796
if (statics && (prop_info->flags & ZEND_ACC_STATIC) != 0) {
37973797
prop = &ce->default_static_members_table[prop_info->offset];
3798+
ZVAL_DEINDIRECT(prop);
37983799
} else if (!statics && (prop_info->flags & ZEND_ACC_STATIC) == 0) {
37993800
prop = &ce->default_properties_table[OBJ_PROP_TO_NUM(prop_info->offset)];
38003801
}
@@ -5503,6 +5504,7 @@ ZEND_METHOD(reflection_property, getValue)
55035504
return;
55045505
}
55055506
member_p = &CE_STATIC_MEMBERS(intern->ce)[ref->prop.offset];
5507+
ZVAL_DEINDIRECT(member_p);
55065508
ZVAL_DEREF(member_p);
55075509
ZVAL_COPY(return_value, member_p);
55085510
} else {
@@ -5570,6 +5572,7 @@ ZEND_METHOD(reflection_property, setValue)
55705572
return;
55715573
}
55725574
variable_ptr = &CE_STATIC_MEMBERS(intern->ce)[ref->prop.offset];
5575+
ZVAL_DEINDIRECT(variable_ptr);
55735576
if (variable_ptr != value) {
55745577
zval garbage;
55755578

‎tests/classes/static_properties_004.phpt

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
--TEST--
2-
Inherited static properties can be separated from their reference set.
2+
Inherited static properties cannot be separated from their reference set.
33
--FILE--
44
<?php
55
class C { public static $p = 'original'; }
@@ -13,7 +13,7 @@ echo "\nChanging one changes all the others:\n";
1313
D::$p = 'changed.all';
1414
var_dump(C::$p, D::$p, E::$p);
1515

16-
echo "\nBut because this is implemented using PHP references, the reference set can easily be split:\n";
16+
echo "\nReferences cannot be used to split the properties:\n";
1717
$ref = 'changed.one';
1818
D::$p =& $ref;
1919
var_dump(C::$p, D::$p, E::$p);
@@ -30,8 +30,8 @@ string(11) "changed.all"
3030
string(11) "changed.all"
3131
string(11) "changed.all"
3232

33-
But because this is implemented using PHP references, the reference set can easily be split:
34-
string(11) "changed.all"
33+
References cannot be used to split the properties:
34+
string(11) "changed.one"
35+
string(11) "changed.one"
3536
string(11) "changed.one"
36-
string(11) "changed.all"
3737
==Done==

0 commit comments

Comments
 (0)