diff --git a/NEWS b/NEWS index c2512379d97..43e6de34c03 100644 --- a/NEWS +++ b/NEWS @@ -18,6 +18,10 @@ PHP NEWS . Fixed bug GH-16593 (Assertion failure in DOM->replaceChild). (nielsdos) . Fixed bug GH-16595 (Another UAF in DOM -> cloneNode). (nielsdos) +- FPM: + . Fixed bug GH-16628 (FPM logs are getting corrupted with this log + statement). (nielsdos) + - GD: . Fixed bug GH-16559 (UBSan abort in ext/gd/libgd/gd_interpolation.c:1007). (nielsdos) @@ -37,6 +41,9 @@ PHP NEWS - PDO_ODBC: . Fixed bug GH-16450 (PDO_ODBC can inject garbage into field values). (cmb) +- Reflection: + . Fixed bug GH-16601 (Memory leak in Reflection constructors). (nielsdos) + - SPL: . Fixed bug GH-16588 (UAF in Observer->serialize). (nielsdos) . Fix GH-16477 (Segmentation fault when calling __debugInfo() after failed diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index e2a7bda46d3..8bf9a7ccebe 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -225,18 +225,26 @@ static void _free_function(zend_function *fptr) /* {{{ */ } /* }}} */ +static void reflection_free_property_reference(property_reference *reference) +{ + zend_string_release_ex(reference->unmangled_name, 0); + efree(reference); +} + +static void reflection_free_parameter_reference(parameter_reference *reference) +{ + _free_function(reference->fptr); + efree(reference); +} + static void reflection_free_objects_storage(zend_object *object) /* {{{ */ { reflection_object *intern = reflection_object_from_obj(object); - parameter_reference *reference; - property_reference *prop_reference; if (intern->ptr) { switch (intern->ref_type) { case REF_TYPE_PARAMETER: - reference = (parameter_reference*)intern->ptr; - _free_function(reference->fptr); - efree(intern->ptr); + reflection_free_parameter_reference(intern->ptr); break; case REF_TYPE_TYPE: { @@ -251,9 +259,7 @@ static void reflection_free_objects_storage(zend_object *object) /* {{{ */ _free_function(intern->ptr); break; case REF_TYPE_PROPERTY: - prop_reference = (property_reference*)intern->ptr; - zend_string_release_ex(prop_reference->unmangled_name, 0); - efree(intern->ptr); + reflection_free_property_reference(intern->ptr); break; case REF_TYPE_ATTRIBUTE: { attribute_reference *attr_ref = intern->ptr; @@ -2585,6 +2591,10 @@ ZEND_METHOD(ReflectionParameter, __construct) } } + if (intern->ptr) { + reflection_free_parameter_reference(intern->ptr); + } + ref = (parameter_reference*) emalloc(sizeof(parameter_reference)); ref->arg_info = &arg_info[position]; ref->offset = (uint32_t)position; @@ -2594,11 +2604,15 @@ ZEND_METHOD(ReflectionParameter, __construct) intern->ptr = ref; intern->ref_type = REF_TYPE_PARAMETER; intern->ce = ce; + zval_ptr_dtor(&intern->obj); if (reference && is_closure) { ZVAL_COPY_VALUE(&intern->obj, reference); + } else { + ZVAL_UNDEF(&intern->obj); } prop_name = reflection_prop_name(object); + zval_ptr_dtor(prop_name); if (has_internal_arg_info(fptr)) { ZVAL_STRING(prop_name, ((zend_internal_arg_info*)arg_info)[position].name); } else { @@ -4129,10 +4143,12 @@ static void reflection_class_object_ctor(INTERNAL_FUNCTION_PARAMETERS, int is_ob object = ZEND_THIS; intern = Z_REFLECTION_P(object); + /* Note: class entry name is interned, no need to destroy them */ if (arg_obj) { ZVAL_STR_COPY(reflection_prop_name(object), arg_obj->ce->name); intern->ptr = arg_obj->ce; if (is_object) { + zval_ptr_dtor(&intern->obj); ZVAL_OBJ_COPY(&intern->obj, arg_obj); } } else { @@ -5819,13 +5835,20 @@ ZEND_METHOD(ReflectionProperty, __construct) } } - ZVAL_STR_COPY(reflection_prop_name(object), name); + zval *prop_name = reflection_prop_name(object); + zval_ptr_dtor(prop_name); + ZVAL_STR_COPY(prop_name, name); + /* Note: class name are always interned, no need to destroy them */ if (dynam_prop == 0) { ZVAL_STR_COPY(reflection_prop_class(object), property_info->ce->name); } else { ZVAL_STR_COPY(reflection_prop_class(object), ce->name); } + if (intern->ptr) { + reflection_free_property_reference(intern->ptr); + } + reference = (property_reference*) emalloc(sizeof(property_reference)); reference->prop = dynam_prop ? NULL : property_info; reference->unmangled_name = zend_string_copy(name); @@ -6670,7 +6693,9 @@ ZEND_METHOD(ReflectionExtension, __construct) RETURN_THROWS(); } free_alloca(lcname, use_heap); - ZVAL_STRING(reflection_prop_name(object), module->name); + zval *prop_name = reflection_prop_name(object); + zval_ptr_dtor(prop_name); + ZVAL_STRING(prop_name, module->name); intern->ptr = module; intern->ref_type = REF_TYPE_OTHER; intern->ce = NULL; diff --git a/ext/reflection/tests/ReflectionExtension_double_construct.phpt b/ext/reflection/tests/ReflectionExtension_double_construct.phpt new file mode 100644 index 00000000000..7bff11c4b17 --- /dev/null +++ b/ext/reflection/tests/ReflectionExtension_double_construct.phpt @@ -0,0 +1,20 @@ +--TEST-- +ReflectionExtension double construct call +--FILE-- +__construct('standard'); +var_dump($r); + +?> +--EXPECT-- +object(ReflectionExtension)#1 (1) { + ["name"]=> + string(8) "standard" +} +object(ReflectionExtension)#1 (1) { + ["name"]=> + string(8) "standard" +} diff --git a/ext/reflection/tests/ReflectionObject_double_construct.phpt b/ext/reflection/tests/ReflectionObject_double_construct.phpt new file mode 100644 index 00000000000..536b145243b --- /dev/null +++ b/ext/reflection/tests/ReflectionObject_double_construct.phpt @@ -0,0 +1,21 @@ +--TEST-- +ReflectionObject double construct call +--FILE-- +__construct($obj); +var_dump($r); + +?> +--EXPECT-- +object(ReflectionObject)#2 (1) { + ["name"]=> + string(8) "stdClass" +} +object(ReflectionObject)#2 (1) { + ["name"]=> + string(8) "stdClass" +} diff --git a/ext/reflection/tests/ReflectionParameter_double_construct.phpt b/ext/reflection/tests/ReflectionParameter_double_construct.phpt new file mode 100644 index 00000000000..cc69e6b10e3 --- /dev/null +++ b/ext/reflection/tests/ReflectionParameter_double_construct.phpt @@ -0,0 +1,27 @@ +--TEST-- +ReflectionParameter double construct call +--FILE-- +__construct($closure, 'x'); +var_dump($r); +$r->__construct('ord', 'character'); +var_dump($r); + +?> +--EXPECT-- +object(ReflectionParameter)#2 (1) { + ["name"]=> + string(1) "x" +} +object(ReflectionParameter)#2 (1) { + ["name"]=> + string(1) "x" +} +object(ReflectionParameter)#2 (1) { + ["name"]=> + string(9) "character" +} diff --git a/ext/reflection/tests/ReflectionProperty_double_construct.phpt b/ext/reflection/tests/ReflectionProperty_double_construct.phpt new file mode 100644 index 00000000000..086f2f781a5 --- /dev/null +++ b/ext/reflection/tests/ReflectionProperty_double_construct.phpt @@ -0,0 +1,24 @@ +--TEST-- +ReflectionProperty double construct call +--FILE-- +__construct(Exception::class, 'message'); +var_dump($r); + +?> +--EXPECT-- +object(ReflectionProperty)#1 (2) { + ["name"]=> + string(7) "message" + ["class"]=> + string(9) "Exception" +} +object(ReflectionProperty)#1 (2) { + ["name"]=> + string(7) "message" + ["class"]=> + string(9) "Exception" +} diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index 1505ea840dc..33dbc36f553 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -830,6 +830,17 @@ static ZEND_FUNCTION(zend_test_is_zend_ptr) RETURN_BOOL(is_zend_ptr((void*)addr)); } +static ZEND_FUNCTION(zend_test_log_err_debug) +{ + zend_string *str; + + ZEND_PARSE_PARAMETERS_START(1, 1) + Z_PARAM_STR(str); + ZEND_PARSE_PARAMETERS_END(); + + php_log_err_with_severity(ZSTR_VAL(str), LOG_DEBUG); +} + static zend_object *zend_test_class_new(zend_class_entry *class_type) { zend_object *obj = zend_objects_new(class_type); diff --git a/ext/zend_test/test.stub.php b/ext/zend_test/test.stub.php index 407c0f344ab..fbfd93da409 100644 --- a/ext/zend_test/test.stub.php +++ b/ext/zend_test/test.stub.php @@ -307,6 +307,8 @@ function zend_test_override_libxml_global_state(): void {} function zend_test_cast_fread($stream): void {} function zend_test_is_zend_ptr(int $addr): bool {} + + function zend_test_log_err_debug(string $str): void {} } namespace ZendTestNS { diff --git a/ext/zend_test/test_arginfo.h b/ext/zend_test/test_arginfo.h index e660a49b4a7..eb83d34fa1f 100644 --- a/ext/zend_test/test_arginfo.h +++ b/ext/zend_test/test_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 5ba6aceffff4e81330182eb84014928630fc9cdd */ + * Stub hash: a7b5de3e4868f9a4aef78da6b98bbce882e129a9 */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_array_return, 0, 0, IS_ARRAY, 0) ZEND_END_ARG_INFO() @@ -165,6 +165,10 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_is_zend_ptr, 0, 1, _IS ZEND_ARG_TYPE_INFO(0, addr, IS_LONG, 0) ZEND_END_ARG_INFO() +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_log_err_debug, 0, 1, IS_VOID, 0) + ZEND_ARG_TYPE_INFO(0, str, IS_STRING, 0) +ZEND_END_ARG_INFO() + #define arginfo_ZendTestNS2_namespaced_func arginfo_zend_test_is_pcre_bundled #define arginfo_ZendTestNS2_namespaced_deprecated_func arginfo_zend_test_void_return @@ -298,6 +302,7 @@ static ZEND_FUNCTION(zend_test_set_fmode); #endif static ZEND_FUNCTION(zend_test_cast_fread); static ZEND_FUNCTION(zend_test_is_zend_ptr); +static ZEND_FUNCTION(zend_test_log_err_debug); static ZEND_FUNCTION(ZendTestNS2_namespaced_func); static ZEND_FUNCTION(ZendTestNS2_namespaced_deprecated_func); static ZEND_FUNCTION(ZendTestNS2_ZendSubNS_namespaced_func); @@ -402,6 +407,7 @@ static const zend_function_entry ext_functions[] = { #endif ZEND_FE(zend_test_cast_fread, arginfo_zend_test_cast_fread) ZEND_FE(zend_test_is_zend_ptr, arginfo_zend_test_is_zend_ptr) + ZEND_FE(zend_test_log_err_debug, arginfo_zend_test_log_err_debug) #if (PHP_VERSION_ID >= 80400) ZEND_RAW_FENTRY(ZEND_NS_NAME("ZendTestNS2", "namespaced_func"), zif_ZendTestNS2_namespaced_func, arginfo_ZendTestNS2_namespaced_func, 0, NULL, NULL) #else diff --git a/sapi/fpm/fpm/zlog.c b/sapi/fpm/fpm/zlog.c index 3903c6eb241..39c6eec885b 100644 --- a/sapi/fpm/fpm/zlog.c +++ b/sapi/fpm/fpm/zlog.c @@ -153,6 +153,7 @@ static inline void zlog_external( } /* }}} */ +/* Returns the length if the print were complete, this can be larger than buf_size. */ static size_t zlog_buf_prefix( const char *function, int line, int flags, char *buf, size_t buf_size, int use_syslog) /* {{{ */ @@ -189,6 +190,7 @@ static size_t zlog_buf_prefix( } } + /* Important: snprintf returns the number of bytes if the print were complete. */ return len; } /* }}} */ @@ -411,6 +413,7 @@ static inline ssize_t zlog_stream_unbuffered_write( static inline ssize_t zlog_stream_buf_copy_cstr( struct zlog_stream *stream, const char *str, size_t str_len) /* {{{ */ { + ZEND_ASSERT(stream->len <= stream->buf.size); if (stream->buf.size - stream->len <= str_len && !zlog_stream_buf_alloc_ex(stream, str_len + stream->len)) { return -1; @@ -425,6 +428,7 @@ static inline ssize_t zlog_stream_buf_copy_cstr( static inline ssize_t zlog_stream_buf_copy_char(struct zlog_stream *stream, char c) /* {{{ */ { + ZEND_ASSERT(stream->len <= stream->buf.size); if (stream->buf.size - stream->len < 1 && !zlog_stream_buf_alloc_ex(stream, 1)) { return -1; } @@ -681,6 +685,17 @@ ssize_t zlog_stream_prefix_ex(struct zlog_stream *stream, const char *function, len = zlog_buf_prefix( function, line, stream->flags, stream->buf.data, stream->buf.size, stream->use_syslog); + if (!EXPECTED(len + 1 <= stream->buf.size)) { + /* If the buffer was not large enough, try with a larger buffer. + * Note that this may still truncate if the zlog_limit is reached. */ + len = MIN(len + 1, zlog_limit); + if (!zlog_stream_buf_alloc_ex(stream, len)) { + return -1; + } + zlog_buf_prefix( + function, line, stream->flags, + stream->buf.data, stream->buf.size, stream->use_syslog); + } stream->len = stream->prefix_len = len; if (stream->msg_prefix != NULL) { zlog_stream_buf_copy_cstr(stream, stream->msg_prefix, stream->msg_prefix_len); @@ -692,8 +707,8 @@ ssize_t zlog_stream_prefix_ex(struct zlog_stream *stream, const char *function, } else { char sbuf[1024]; ssize_t written; - len = zlog_buf_prefix(function, line, stream->flags, sbuf, 1024, stream->use_syslog); - written = zlog_stream_direct_write(stream, sbuf, len); + len = zlog_buf_prefix(function, line, stream->flags, sbuf, sizeof(sbuf), stream->use_syslog); + written = zlog_stream_direct_write(stream, sbuf, MIN(len, sizeof(sbuf))); if (stream->msg_prefix != NULL) { written += zlog_stream_direct_write( stream, stream->msg_prefix, stream->msg_prefix_len); diff --git a/sapi/fpm/tests/gh16628.phpt b/sapi/fpm/tests/gh16628.phpt new file mode 100644 index 00000000000..e2df0c8cb84 --- /dev/null +++ b/sapi/fpm/tests/gh16628.phpt @@ -0,0 +1,53 @@ +--TEST-- +GH-16628 (FPM logs are getting corrupted with this log statement) +--EXTENSIONS-- +zend_test +--SKIPIF-- + +--FILE-- +start(); +$tester->expectLogStartNotices(); +$tester->request()->expectEmptyBody(); +for ($i = 1; $i < 100; $i++) { + $tester->expectLogNotice("%sPHP message: " . str_repeat("a", $i)); +} +$tester->terminate(); +$tester->expectLogTerminatingNotices(); +$tester->close(); + +?> +Done +--EXPECT-- +Done +--CLEAN-- +