From 046fce282c1c33045abd1d2ba2707222ab4e709b Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 10 Jan 2023 14:55:19 +0100 Subject: [PATCH 1/3] Prevent direct instantiation of com_safearray_proxy The `com_safearray_proxy` class is meant for internal usage, but so far it was possible to instantiate it from userland, although that made no sense. However, a while ago there was a relevant change[1], namely that its `default_object_handlers` are now assigned when the class is registered, while previously they only have been assigned when an instance had been created internally. So now when freeing a manually created object, `free_obj()` is called, although the object never has been properly initialized (causing segfaults). We fix this by introducing a `create_object()` handler which properly initializes the object with dummy values. Since a manually created `com_safearray_proxy` still does not make sense, we disallow its instantiation. [1] --- ext/com_dotnet/com_extension.c | 1 + ext/com_dotnet/com_saproxy.c | 16 ++++++++++++++-- ext/com_dotnet/php_com_dotnet_internal.h | 1 + ext/com_dotnet/tests/saproxy_instantiate.phpt | 14 ++++++++++++++ 4 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 ext/com_dotnet/tests/saproxy_instantiate.phpt diff --git a/ext/com_dotnet/com_extension.c b/ext/com_dotnet/com_extension.c index 1f18d2a8573ab..df8314fa7613e 100644 --- a/ext/com_dotnet/com_extension.c +++ b/ext/com_dotnet/com_extension.c @@ -182,6 +182,7 @@ PHP_MINIT_FUNCTION(com_dotnet) php_com_saproxy_class_entry = register_class_com_safearray_proxy(); /* php_com_saproxy_class_entry->constructor->common.fn_flags |= ZEND_ACC_PROTECTED; */ + php_com_saproxy_class_entry->create_object = php_com_saproxy_create_object; php_com_saproxy_class_entry->default_object_handlers = &php_com_saproxy_handlers; php_com_saproxy_class_entry->get_iterator = php_com_saproxy_iter_get; diff --git a/ext/com_dotnet/com_saproxy.c b/ext/com_dotnet/com_saproxy.c index c68001b1b311f..50b94e2b07620 100644 --- a/ext/com_dotnet/com_saproxy.c +++ b/ext/com_dotnet/com_saproxy.c @@ -57,6 +57,16 @@ typedef struct { #define SA_FETCH(zv) (php_com_saproxy*)Z_OBJ_P(zv) +zend_object *php_com_saproxy_create_object(zend_class_entry *class_type) { + php_com_saproxy *intern = zend_object_alloc(sizeof(php_com_saproxy), class_type); + memset((char*)intern + sizeof(zend_object), 0, sizeof(php_com_saproxy) - sizeof(zend_object)); + + zend_object_std_init(&intern->std, class_type); + object_properties_init(&intern->std, class_type); + + return &intern->std; +} + static inline void clone_indices(php_com_saproxy *dest, php_com_saproxy *src, int ndims) { int i; @@ -317,7 +327,7 @@ static zend_function *saproxy_method_get(zend_object **object, zend_string *name static zend_function *saproxy_constructor_get(zend_object *object) { - /* user cannot instantiate */ + zend_throw_error(NULL, "Cannot directly construct com_safeproxy_array; it is for internal usage only"); return NULL; } @@ -365,7 +375,9 @@ static void saproxy_free_storage(zend_object *object) //??? } //??? } - OBJ_RELEASE(&proxy->obj->zo); + if (&proxy->obj->zo != NULL) { + OBJ_RELEASE(&proxy->obj->zo); + } zend_object_std_dtor(object); diff --git a/ext/com_dotnet/php_com_dotnet_internal.h b/ext/com_dotnet/php_com_dotnet_internal.h index 6735afba94289..90c3ab2d40e41 100644 --- a/ext/com_dotnet/php_com_dotnet_internal.h +++ b/ext/com_dotnet/php_com_dotnet_internal.h @@ -76,6 +76,7 @@ extern zend_object_handlers php_com_object_handlers; void php_com_object_enable_event_sink(php_com_dotnet_object *obj, bool enable); /* com_saproxy.c */ +zend_object *php_com_saproxy_create_object(zend_class_entry *class_type); zend_object_iterator *php_com_saproxy_iter_get(zend_class_entry *ce, zval *object, int by_ref); void php_com_saproxy_create(zend_object *com_object, zval *proxy_out, zval *index); extern zend_object_handlers php_com_saproxy_handlers; diff --git a/ext/com_dotnet/tests/saproxy_instantiate.phpt b/ext/com_dotnet/tests/saproxy_instantiate.phpt new file mode 100644 index 0000000000000..303305add09e2 --- /dev/null +++ b/ext/com_dotnet/tests/saproxy_instantiate.phpt @@ -0,0 +1,14 @@ +--TEST-- +Manual instantiation of com_safearray_proxy is not allowed +--EXTENSIONS-- +com_dotnet +--FILE-- +getMessage(), PHP_EOL; +} +?> +--EXPECT-- +Cannot directly construct com_safeproxy_array; it is for internal usage only From e5e8d178e829f8178700110799987ffb49f848a5 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Sun, 6 Oct 2024 19:12:35 +0200 Subject: [PATCH 2/3] Fix object instantiation com_dotnet objects are special, since they still have the `zend_object` as first member, so we need to use a somewhat different initialization. --- ext/com_dotnet/com_saproxy.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/ext/com_dotnet/com_saproxy.c b/ext/com_dotnet/com_saproxy.c index 50b94e2b07620..771e8d06138c9 100644 --- a/ext/com_dotnet/com_saproxy.c +++ b/ext/com_dotnet/com_saproxy.c @@ -57,13 +57,11 @@ typedef struct { #define SA_FETCH(zv) (php_com_saproxy*)Z_OBJ_P(zv) -zend_object *php_com_saproxy_create_object(zend_class_entry *class_type) { - php_com_saproxy *intern = zend_object_alloc(sizeof(php_com_saproxy), class_type); - memset((char*)intern + sizeof(zend_object), 0, sizeof(php_com_saproxy) - sizeof(zend_object)); - +zend_object *php_com_saproxy_create_object(zend_class_entry *class_type) +{ + php_com_saproxy *intern = emalloc(sizeof(*intern)); + memset(intern, 0, sizeof(*intern)); zend_object_std_init(&intern->std, class_type); - object_properties_init(&intern->std, class_type); - return &intern->std; } From af6b42916a4f40aa96217ec9ffd952691a69bd5a Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Sun, 6 Oct 2024 23:52:50 +0200 Subject: [PATCH 3/3] Update ext/com_dotnet/com_saproxy.c Co-authored-by: Niels Dossche <7771979+nielsdos@users.noreply.github.com> --- ext/com_dotnet/com_saproxy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/com_dotnet/com_saproxy.c b/ext/com_dotnet/com_saproxy.c index 771e8d06138c9..ea0e9e47a13d9 100644 --- a/ext/com_dotnet/com_saproxy.c +++ b/ext/com_dotnet/com_saproxy.c @@ -373,7 +373,7 @@ static void saproxy_free_storage(zend_object *object) //??? } //??? } - if (&proxy->obj->zo != NULL) { + if (proxy->obj != NULL) { OBJ_RELEASE(&proxy->obj->zo); }