stream_wrapper_register crashes with FFI\CData provided as class. Forbid it for some/all classes with custom handlers?

[TysonAndre]

Description

The following code:

<?php
stream_wrapper_register('badffi', 'FFI\CData');
file_get_contents('badffi://x');

Resulted in this output:

segmentation fault (core dumped)

==80156== Invalid read of size 4
==80156==    at 0x4B6334: zend_ffi_cdata_write_field (ffi.c:1290)
==80156==    by 0xA7D376: add_property_zval_ex (zend_API.c:2183)
==80156==    by 0xA7CEE3: add_property_resource_ex (zend_API.c:2104)
==80156==    by 0x9F8C19: add_property_resource (zend_API.h:640)
==80156==    by 0x9F9131: user_stream_create_object (userspace.c:302)
==80156==    by 0x9F92FC: user_wrapper_opener (userspace.c:345)
==80156==    by 0x9EFC2C: _php_stream_open_wrapper_ex (streams.c:2167)
==80156==    by 0x889A9A: zif_file_get_contents (file.c:426)
==80156==    by 0x7059BD: phar_file_get_contents (func_interceptors.c:227)
==80156==    by 0xAB4919: ZEND_DO_ICALL_SPEC_RETVAL_UNUSED_HANDLER (zend_vm_execute.h:1250)
==80156==    by 0xB2B883: execute_ex (zend_vm_execute.h:55975)
==80156==    by 0xB310D5: zend_execute (zend_vm_execute.h:60343)
==80156==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==80156== 
==80156== 
==80156== Process terminating with default action of signal 11 (SIGSEGV)
==80156==  Access not within mapped region at address 0x0
==80156==    at 0x4B6334: zend_ffi_cdata_write_field (ffi.c:1290)
==80156==    by 0xA7D376: add_property_zval_ex (zend_API.c:2183)
==80156==    by 0xA7CEE3: add_property_resource_ex (zend_API.c:2104)
==80156==    by 0x9F8C19: add_property_resource (zend_API.h:640)
==80156==    by 0x9F9131: user_stream_create_object (userspace.c:302)
==80156==    by 0x9F92FC: user_wrapper_opener (userspace.c:345)
==80156==    by 0x9EFC2C: _php_stream_open_wrapper_ex (streams.c:2167)
==80156==    by 0x889A9A: zif_file_get_contents (file.c:426)
==80156==    by 0x7059BD: phar_file_get_contents (func_interceptors.c:227)
==80156==    by 0xAB4919: ZEND_DO_ICALL_SPEC_RETVAL_UNUSED_HANDLER (zend_vm_execute.h:1250)
==80156==    by 0xB2B883: execute_ex (zend_vm_execute.h:55975)
==80156==    by 0xB310D5: zend_execute (zend_vm_execute.h:60343)

But I expected this output instead:

// A thrown error for attempting to call the constructor of CData

---

Other notes:

1. Stream wrappers work by adding dynamic properties to a class
2. This could check obj->handlers for write_property, get_properties, get_properties_for, and read_property overrides?
3. It might make sense to check if the constructor succeeded (EG(exception) == NULL) before setting context. FFI crashes because the object isn't initialized. That may be a bc break if any constructors check $this->context
   Maybe that should be done that way only for internal classes and their subclasses in older php versions, to minimize the impact
4. (new ReflectionClass('FFI\CData'))->newInstanceWithoutConstructor() throws for internal classes right now - but stream wrappers do call constructors (streams are just modifying the object before calling the constructor)

// main/streams/userspace.c
static void user_stream_create_object(struct php_user_stream_wrapper *uwrap, php_stream_context *context, zval *object)
{
	if (uwrap->ce->ce_flags & (ZEND_ACC_INTERFACE|ZEND_ACC_TRAIT|ZEND_ACC_IMPLICIT_ABSTRACT_CLASS|ZEND_ACC_EXPLICIT_ABSTRACT_CLASS)) {
		ZVAL_UNDEF(object);
		return;
	}

	/* create an instance of our class */
	if (object_init_ex(object, uwrap->ce) == FAILURE) {
		ZVAL_UNDEF(object);
		return;
	}

	if (context) {
		GC_ADDREF(context->res);
		add_property_resource(object, "context", context->res);
	} else {
		add_property_null(object, "context");
	}

	if (uwrap->ce->constructor) {
		zend_call_known_instance_method_with_0_params(
			uwrap->ce->constructor, Z_OBJ_P(object), NULL);
	}
}

Noticed while looking into #9697 but they have different causes

PHP Version

7.4-8.3-dev

Operating System

No response

[cmb69]

1. Stream wrappers work by adding dynamic properties to a class

What doesn't look like a good thing, but needs to be documented, especially wrt. the deprecation of dynamic properties.

Anyhow, I'm not sure how much effort we want to spend on the current behavior; in my opinion, userland stream wrappers should implement an interface, in which case internal classes couldn't be used as stream wrappers, unless they'd implement that interface.