Permalink
Browse files

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.
  • Loading branch information...
nikic committed Jun 22, 2018
1 parent 102bcb5 commit 2543e61aed67add7522e0b4cdf9a13cf3e441f6f
2 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
@@ -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:
@@ -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);
}
@@ -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)];
}
@@ -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)];
}
@@ -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;
}
/* }}} */
@@ -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); \
@@ -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);
@@ -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);
@@ -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]);
}
}
@@ -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);
@@ -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;
@@ -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==

0 comments on commit 2543e61

Please sign in to comment.