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.
This commit is contained in:
Nikita Popov 2018-06-22 12:58:48 +02:00
parent 102bcb5c05
commit 2543e61aed
13 changed files with 105 additions and 57 deletions

2
NEWS
View file

@ -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

View file

@ -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:

View file

@ -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);
}

View file

@ -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)];
}

View file

@ -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)];
}

View file

@ -1445,7 +1445,6 @@ 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)) {
@ -1453,9 +1452,11 @@ 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;
}
/* }}} */

View file

@ -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); \

View file

@ -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);

View file

@ -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);

View file

@ -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]);
}
}

View file

@ -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);

View file

@ -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;

View file

@ -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==