Repository navigation
stream_wrapper_register crashes with FFI\CData provided as class. Forbid it for some/all classes with custom handlers? #9698
Description
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:
- Stream wrappers work by adding dynamic properties to a class
- This could check obj->handlers for write_property, get_properties, get_properties_for, and read_property overrides?
- 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 (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
Activity
cmb69 commentedon Oct 10, 2022
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.