Skip to content

Commit

Permalink
Add flag to forbid dynamic property creation on internal classes
Browse files Browse the repository at this point in the history
While performing resource -> object migrations, we're adding
defensive classes that are final, non-serializable and non-clonable
(unless they are, of course). This path adds a ZEND_ACC_NO_DYNAMIC_PROPERTIES
flag, that also forbids the creation of dynamic properties on these objects.
This is a subset of #3931 and targeted at internal usage only
(though may be extended to userland at some point in the future).

It's already possible to achieve this (what the removed
WeakRef/WeakMap code does), but there's some caveats: First, this
simple approach is only possible if the class has no declared
properties, otherwise it's necessary to special-case those
properties. Second, it's easy to make it overly strict, e.g. by
forbidding isset($obj->prop) as well. And finally, it requires a
lot of boilerplate code for each class.

Closes GH-5572.
  • Loading branch information
nikic committed Jun 24, 2020
1 parent 2a28589 commit 653e4ea
Show file tree
Hide file tree
Showing 13 changed files with 53 additions and 97 deletions.
30 changes: 13 additions & 17 deletions Zend/tests/weakrefs/weakmap_error_conditions.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,17 @@ try {
echo $e->getMessage(), "\n";
}

var_dump($map->prop);
var_dump(isset($map->prop));
unset($map->prop);

try {
$map->prop = 1;
} catch (Error $e) {
echo $e->getMessage(), "\n";
}
try {
var_dump($map->prop);
$map->prop[] = 1;
} catch (Error $e) {
echo $e->getMessage(), "\n";
}
Expand All @@ -56,16 +60,6 @@ try {
} catch (Error $e) {
echo $e->getMessage(), "\n";
}
try {
isset($map->prop);
} catch (Error $e) {
echo $e->getMessage(), "\n";
}
try {
unset($map->prop);
} catch (Error $e) {
echo $e->getMessage(), "\n";
}

try {
serialize($map);
Expand All @@ -79,18 +73,20 @@ try {
}

?>
--EXPECT--
--EXPECTF--
WeakMap key must be an object
WeakMap key must be an object
WeakMap key must be an object
WeakMap key must be an object
Cannot append to WeakMap
Cannot append to WeakMap
Object stdClass#2 not contained in WeakMap
WeakMap objects do not support properties
WeakMap objects do not support properties
WeakMap objects do not support property references
WeakMap objects do not support properties
WeakMap objects do not support properties

Warning: Undefined property: WeakMap::$prop in %s on line %d
NULL
bool(false)
Cannot create dynamic property WeakMap::$prop
Cannot create dynamic property WeakMap::$prop
Cannot create dynamic property WeakMap::$prop
Serialization of 'WeakMap' is not allowed
Unserialization of 'WeakMap' is not allowed
33 changes: 9 additions & 24 deletions Zend/tests/weakrefs/weakrefs_003.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -4,40 +4,25 @@ WeakReference object handlers
<?php
$wr = WeakReference::create(new stdClass);

try {
$wr->disallow;
} catch (Error $ex) {
var_dump($ex->getMessage());
}
var_dump($wr->disallow);
var_dump(isset($wr->disallow));
unset($wr->disallow);

try {
$wr->disallow = "writes";
} catch (Error $ex) {
var_dump($ex->getMessage());
}

try {
isset($wr->disallow);
} catch (Error $ex) {
var_dump($ex->getMessage());
}

try {
unset($wr->disallow);
} catch (Error $ex) {
var_dump($ex->getMessage());
}

try {
$disallow = &$wr->disallowed;
} catch (Error $ex) {
var_dump($ex->getMessage());
}
?>
--EXPECT--
string(47) "WeakReference objects do not support properties"
string(47) "WeakReference objects do not support properties"
string(47) "WeakReference objects do not support properties"
string(47) "WeakReference objects do not support properties"
string(56) "WeakReference objects do not support property references"

--EXPECTF--
Warning: Undefined property: WeakReference::$disallow in %s on line %d
NULL
bool(false)
string(55) "Cannot create dynamic property WeakReference::$disallow"
string(57) "Cannot create dynamic property WeakReference::$disallowed"
5 changes: 4 additions & 1 deletion Zend/zend_compile.h
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ typedef struct _zend_oparray_context {
/* op_array or class is preloaded | | | */
#define ZEND_ACC_PRELOADED (1 << 10) /* X | X | | */
/* | | | */
/* Class Flags (unused: 13, 14, 15, 24...) | | | */
/* Class Flags (unused: 14, 15, 24...) | | | */
/* =========== | | | */
/* | | | */
/* Special class types | | | */
Expand All @@ -260,6 +260,9 @@ typedef struct _zend_oparray_context {
/* Class constants updated | | | */
#define ZEND_ACC_CONSTANTS_UPDATED (1 << 12) /* X | | | */
/* | | | */
/* Objects of this class may not have dynamic properties | | | */
#define ZEND_ACC_NO_DYNAMIC_PROPERTIES (1 << 13) /* X | | | */
/* | | | */
/* User class has methods with static variables | | | */
#define ZEND_HAS_STATIC_IN_METHODS (1 << 16) /* X | | | */
/* | | | */
Expand Down
15 changes: 15 additions & 0 deletions Zend/zend_object_handlers.c
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,12 @@ static ZEND_COLD zend_never_inline void zend_bad_property_name(void) /* {{{ */
}
/* }}} */

static ZEND_COLD zend_never_inline void zend_forbidden_dynamic_property(
zend_class_entry *ce, zend_string *member) {
zend_throw_error(NULL, "Cannot create dynamic property %s::$%s",
ZSTR_VAL(ce->name), ZSTR_VAL(member));
}

static zend_always_inline uintptr_t zend_get_property_offset(zend_class_entry *ce, zend_string *member, int silent, void **cache_slot, zend_property_info **info_ptr) /* {{{ */
{
zval *zv;
Expand Down Expand Up @@ -772,6 +778,11 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva

ZVAL_COPY_VALUE(variable_ptr, value);
} else {
if (UNEXPECTED(zobj->ce->ce_flags & ZEND_ACC_NO_DYNAMIC_PROPERTIES)) {
zend_forbidden_dynamic_property(zobj->ce, name);
variable_ptr = &EG(error_zval);
goto exit;
}
if (!zobj->properties) {
rebuild_object_properties(zobj);
}
Expand Down Expand Up @@ -935,6 +946,10 @@ ZEND_API zval *zend_std_get_property_ptr_ptr(zend_object *zobj, zend_string *nam
}
if (EXPECTED(!zobj->ce->__get) ||
UNEXPECTED((*zend_get_property_guard(zobj, name)) & IN_GET)) {
if (UNEXPECTED(zobj->ce->ce_flags & ZEND_ACC_NO_DYNAMIC_PROPERTIES)) {
zend_forbidden_dynamic_property(zobj->ce, name);
return &EG(error_zval);
}
if (UNEXPECTED(!zobj->properties)) {
rebuild_object_properties(zobj);
}
Expand Down
47 changes: 2 additions & 45 deletions Zend/zend_weakrefs.c
Original file line number Diff line number Diff line change
Expand Up @@ -234,39 +234,6 @@ static void zend_weakref_free(zend_object *zo) {
zend_object_std_dtor(&wr->std);
}

#define zend_weakref_unsupported(object, thing) \
zend_throw_error(NULL, "%s objects do not support " thing, ZSTR_VAL(object->ce->name));

static ZEND_COLD zval* zend_weakref_no_write(zend_object *object, zend_string *member, zval *value, void **rtc) {
zend_weakref_unsupported(object, "properties");

return &EG(uninitialized_zval);
}

static ZEND_COLD zval* zend_weakref_no_read(zend_object *object, zend_string *member, int type, void **rtc, zval *rv) {
if (!EG(exception)) {
zend_weakref_unsupported(object, "properties");
}

return &EG(uninitialized_zval);
}

static ZEND_COLD zval *zend_weakref_no_read_ptr(zend_object *object, zend_string *member, int type, void **rtc) {
zend_weakref_unsupported(object, "property references");
return NULL;
}

static ZEND_COLD int zend_weakref_no_isset(zend_object *object, zend_string *member, int hse, void **rtc) {
if (hse != 2) {
zend_weakref_unsupported(object, "properties");
}
return 0;
}

static ZEND_COLD void zend_weakref_no_unset(zend_object *object, zend_string *member, void **rtc) {
zend_weakref_unsupported(object, "properties");
}

ZEND_COLD ZEND_METHOD(WeakReference, __construct)
{
zend_throw_error(NULL,
Expand Down Expand Up @@ -615,7 +582,7 @@ void zend_register_weakref_ce(void) /* {{{ */

INIT_CLASS_ENTRY(ce, "WeakReference", class_WeakReference_methods);
zend_ce_weakref = zend_register_internal_class(&ce);
zend_ce_weakref->ce_flags |= ZEND_ACC_FINAL;
zend_ce_weakref->ce_flags |= ZEND_ACC_FINAL | ZEND_ACC_NO_DYNAMIC_PROPERTIES;

zend_ce_weakref->create_object = zend_weakref_new;
zend_ce_weakref->serialize = zend_class_serialize_deny;
Expand All @@ -625,16 +592,11 @@ void zend_register_weakref_ce(void) /* {{{ */
zend_weakref_handlers.offset = XtOffsetOf(zend_weakref, std);

zend_weakref_handlers.free_obj = zend_weakref_free;
zend_weakref_handlers.read_property = zend_weakref_no_read;
zend_weakref_handlers.write_property = zend_weakref_no_write;
zend_weakref_handlers.has_property = zend_weakref_no_isset;
zend_weakref_handlers.unset_property = zend_weakref_no_unset;
zend_weakref_handlers.get_property_ptr_ptr = zend_weakref_no_read_ptr;
zend_weakref_handlers.clone_obj = NULL;

INIT_CLASS_ENTRY(ce, "WeakMap", class_WeakMap_methods);
zend_ce_weakmap = zend_register_internal_class(&ce);
zend_ce_weakmap->ce_flags |= ZEND_ACC_FINAL;
zend_ce_weakmap->ce_flags |= ZEND_ACC_FINAL | ZEND_ACC_NO_DYNAMIC_PROPERTIES;

zend_ce_weakmap->create_object = zend_weakmap_create_object;
zend_ce_weakmap->get_iterator = zend_weakmap_get_iterator;
Expand All @@ -656,11 +618,6 @@ void zend_register_weakref_ce(void) /* {{{ */
zend_weakmap_handlers.get_properties_for = zend_weakmap_get_properties_for;
zend_weakmap_handlers.get_gc = zend_weakmap_get_gc;
zend_weakmap_handlers.clone_obj = zend_weakmap_clone_obj;
zend_weakmap_handlers.read_property = zend_weakref_no_read;
zend_weakmap_handlers.write_property = zend_weakref_no_write;
zend_weakmap_handlers.has_property = zend_weakref_no_isset;
zend_weakmap_handlers.unset_property = zend_weakref_no_unset;
zend_weakmap_handlers.get_property_ptr_ptr = zend_weakref_no_read_ptr;
}
/* }}} */

2 changes: 1 addition & 1 deletion ext/curl/interface.c
Original file line number Diff line number Diff line change
Expand Up @@ -1193,7 +1193,7 @@ PHP_MINIT_FUNCTION(curl)
zend_class_entry ce;
INIT_CLASS_ENTRY(ce, "CurlHandle", class_CurlHandle_methods);
curl_ce = zend_register_internal_class(&ce);
curl_ce->ce_flags |= ZEND_ACC_FINAL;
curl_ce->ce_flags |= ZEND_ACC_FINAL | ZEND_ACC_NO_DYNAMIC_PROPERTIES;
curl_ce->create_object = curl_create_object;
curl_ce->serialize = zend_class_serialize_deny;
curl_ce->unserialize = zend_class_unserialize_deny;
Expand Down
2 changes: 1 addition & 1 deletion ext/curl/multi.c
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ void curl_multi_register_class(const zend_function_entry *method_entries) {
zend_class_entry ce_multi;
INIT_CLASS_ENTRY(ce_multi, "CurlMultiHandle", method_entries);
curl_multi_ce = zend_register_internal_class(&ce_multi);
curl_multi_ce->ce_flags |= ZEND_ACC_FINAL;
curl_multi_ce->ce_flags |= ZEND_ACC_FINAL | ZEND_ACC_NO_DYNAMIC_PROPERTIES;
curl_multi_ce->create_object = curl_multi_create_object;
curl_multi_ce->serialize = zend_class_serialize_deny;
curl_multi_ce->unserialize = zend_class_unserialize_deny;
Expand Down
2 changes: 1 addition & 1 deletion ext/curl/share.c
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ void curl_share_register_class(const zend_function_entry *method_entries) {
zend_class_entry ce_share;
INIT_CLASS_ENTRY(ce_share, "CurlShareHandle", method_entries);
curl_share_ce = zend_register_internal_class(&ce_share);
curl_share_ce->ce_flags |= ZEND_ACC_FINAL;
curl_share_ce->ce_flags |= ZEND_ACC_FINAL | ZEND_ACC_NO_DYNAMIC_PROPERTIES;
curl_share_ce->create_object = curl_share_create_object;
curl_share_ce->serialize = &zend_class_serialize_deny;
curl_share_ce->unserialize = &zend_class_unserialize_deny;
Expand Down
4 changes: 2 additions & 2 deletions ext/enchant/enchant.c
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ PHP_MINIT_FUNCTION(enchant)

INIT_CLASS_ENTRY(bce, "EnchantBroker", class_EnchantBroker_methods);
enchant_broker_ce = zend_register_internal_class(&bce);
enchant_broker_ce->ce_flags |= ZEND_ACC_FINAL;
enchant_broker_ce->ce_flags |= ZEND_ACC_FINAL | ZEND_ACC_NO_DYNAMIC_PROPERTIES;
enchant_broker_ce->create_object = enchant_broker_create_object;
enchant_broker_ce->serialize = zend_class_serialize_deny;
enchant_broker_ce->unserialize = zend_class_unserialize_deny;
Expand All @@ -204,7 +204,7 @@ PHP_MINIT_FUNCTION(enchant)

INIT_CLASS_ENTRY(dce, "EnchantDictionary", class_EnchantDictionary_methods);
enchant_dict_ce = zend_register_internal_class(&dce);
enchant_dict_ce->ce_flags |= ZEND_ACC_FINAL;
enchant_dict_ce->ce_flags |= ZEND_ACC_FINAL | ZEND_ACC_NO_DYNAMIC_PROPERTIES;
enchant_dict_ce->create_object = enchant_dict_create_object;
enchant_dict_ce->serialize = zend_class_serialize_deny;
enchant_dict_ce->unserialize = zend_class_unserialize_deny;
Expand Down
2 changes: 1 addition & 1 deletion ext/sysvsem/sysvsem.c
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ PHP_MINIT_FUNCTION(sysvsem)
zend_class_entry ce;
INIT_CLASS_ENTRY(ce, "SysvSemaphore", class_SysvSemaphore_methods);
sysvsem_ce = zend_register_internal_class(&ce);
sysvsem_ce->ce_flags |= ZEND_ACC_FINAL;
sysvsem_ce->ce_flags |= ZEND_ACC_FINAL | ZEND_ACC_NO_DYNAMIC_PROPERTIES;
sysvsem_ce->create_object = sysvsem_create_object;
sysvsem_ce->serialize = zend_class_serialize_deny;
sysvsem_ce->unserialize = zend_class_unserialize_deny;
Expand Down
2 changes: 1 addition & 1 deletion ext/sysvshm/sysvshm.c
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ PHP_MINIT_FUNCTION(sysvshm)
zend_class_entry ce;
INIT_CLASS_ENTRY(ce, "SysvSharedMemory", class_SysvSharedMemory_methods);
sysvshm_ce = zend_register_internal_class(&ce);
sysvshm_ce->ce_flags |= ZEND_ACC_FINAL;
sysvshm_ce->ce_flags |= ZEND_ACC_FINAL | ZEND_ACC_NO_DYNAMIC_PROPERTIES;
sysvshm_ce->create_object = sysvshm_create_object;
sysvshm_ce->serialize = zend_class_serialize_deny;
sysvshm_ce->unserialize = zend_class_unserialize_deny;
Expand Down
2 changes: 1 addition & 1 deletion ext/xml/xml.c
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ PHP_MINIT_FUNCTION(xml)
INIT_CLASS_ENTRY(ce, "XmlParser", class_XMLParser_methods);
xml_parser_ce = zend_register_internal_class(&ce);
xml_parser_ce->create_object = xml_parser_create_object;
xml_parser_ce->ce_flags |= ZEND_ACC_FINAL;
xml_parser_ce->ce_flags |= ZEND_ACC_FINAL | ZEND_ACC_NO_DYNAMIC_PROPERTIES;
xml_parser_ce->serialize = zend_class_serialize_deny;
xml_parser_ce->unserialize = zend_class_unserialize_deny;

Expand Down
4 changes: 2 additions & 2 deletions ext/zlib/zlib.c
Original file line number Diff line number Diff line change
Expand Up @@ -1359,7 +1359,7 @@ static PHP_MINIT_FUNCTION(zlib)
zend_class_entry inflate_ce;
INIT_CLASS_ENTRY(inflate_ce, "InflateContext", class_InflateContext_methods);
inflate_context_ce = zend_register_internal_class(&inflate_ce);
inflate_context_ce->ce_flags |= ZEND_ACC_FINAL;
inflate_context_ce->ce_flags |= ZEND_ACC_FINAL | ZEND_ACC_NO_DYNAMIC_PROPERTIES;
inflate_context_ce->create_object = inflate_context_create_object;
inflate_context_ce->serialize = zend_class_serialize_deny;
inflate_context_ce->unserialize = zend_class_unserialize_deny;
Expand All @@ -1373,7 +1373,7 @@ static PHP_MINIT_FUNCTION(zlib)
zend_class_entry deflate_ce;
INIT_CLASS_ENTRY(deflate_ce, "DeflateContext", class_DeflateContext_methods);
deflate_context_ce = zend_register_internal_class(&deflate_ce);
deflate_context_ce->ce_flags |= ZEND_ACC_FINAL;
deflate_context_ce->ce_flags |= ZEND_ACC_FINAL | ZEND_ACC_NO_DYNAMIC_PROPERTIES;
deflate_context_ce->create_object = deflate_context_create_object;
deflate_context_ce->serialize = zend_class_serialize_deny;
deflate_context_ce->unserialize = zend_class_unserialize_deny;
Expand Down

0 comments on commit 653e4ea

Please sign in to comment.