From ef9790a5f1374c2d88fd36e25db98b6bd0dfc47d Mon Sep 17 00:00:00 2001 From: ShadowRider09 Date: Wed, 12 Nov 2025 18:29:12 +0800 Subject: [PATCH] Fix multiple memory safety issues detected by ASan/UBSan/LeakSan Fix null pointer dereference in filter_avcolour_space.c when profile is NULL Fix Pango/Fontconfig memory leak in producer_pango.c by adding instance counting Fix null pointer dereferences in producer_colour.c (two locations) Fix null pointer dereference in mlt_producer_set_in_and_out Fix null pointer dereference in filter_movit_convert.cpp Fix double-free in producer_xml.c by properly handling stack state All fixes have been verified with fuzzing tests. See FIXES_REPORT.md for details, and all of the crash is in the mlt/fuzz/fixed_crashes --- src/framework/mlt_producer.c | 4 +++ src/modules/avformat/filter_avcolour_space.c | 9 +++--- src/modules/core/producer_colour.c | 31 +++++++++++++++----- src/modules/gdk/producer_pango.c | 17 +++++++++++ src/modules/movit/filter_movit_convert.cpp | 10 +++---- src/modules/xml/producer_xml.c | 21 ++++++++++++- 6 files changed, 75 insertions(+), 17 deletions(-) diff --git a/src/framework/mlt_producer.c b/src/framework/mlt_producer.c index 904c9e029..5241bf379 100644 --- a/src/framework/mlt_producer.c +++ b/src/framework/mlt_producer.c @@ -486,6 +486,10 @@ double mlt_producer_get_fps(mlt_producer self) int mlt_producer_set_in_and_out(mlt_producer self, mlt_position in, mlt_position out) { + + if (self == NULL) { + return 1; + } if (self->set_in_and_out) { return self->set_in_and_out(self, in, out); } diff --git a/src/modules/avformat/filter_avcolour_space.c b/src/modules/avformat/filter_avcolour_space.c index 72cd92e50..2fb592f08 100644 --- a/src/modules/avformat/filter_avcolour_space.c +++ b/src/modules/avformat/filter_avcolour_space.c @@ -348,10 +348,11 @@ static mlt_frame filter_process(mlt_filter filter, mlt_frame frame) // The producer may still change it during get_image. // This way we do not have to modify each producer to set a valid colorspace. mlt_properties properties = MLT_FRAME_PROPERTIES(frame); - if (mlt_properties_get_int(properties, "colorspace") <= 0) - mlt_properties_set_int(properties, - "colorspace", - mlt_service_profile(MLT_FILTER_SERVICE(filter))->colorspace); + if (mlt_properties_get_int(properties, "colorspace") <= 0) { + mlt_profile profile = mlt_service_profile(MLT_FILTER_SERVICE(filter)); + int colorspace = profile ? profile->colorspace : mlt_colorspace_bt601; + mlt_properties_set_int(properties, "colorspace", colorspace); + } if (!frame->convert_image) frame->convert_image = convert_image; diff --git a/src/modules/core/producer_colour.c b/src/modules/core/producer_colour.c index 81becaeb6..47870f468 100644 --- a/src/modules/core/producer_colour.c +++ b/src/modules/core/producer_colour.c @@ -95,10 +95,21 @@ static int producer_get_image(mlt_frame frame, // Choose suitable out values if nothing specific requested if (*format == mlt_image_none || *format == mlt_image_movit) *format = mlt_image_rgba; - if (*width <= 0) - *width = mlt_service_profile(MLT_PRODUCER_SERVICE(producer))->width; - if (*height <= 0) - *height = mlt_service_profile(MLT_PRODUCER_SERVICE(producer))->height; + if (*width <= 0 || *height <= 0) { + mlt_profile profile = mlt_service_profile(MLT_PRODUCER_SERVICE(producer)); + if (profile) { + if (*width <= 0) + *width = profile->width; + if (*height <= 0) + *height = profile->height; + } else { + if (*width <= 0) + *width = 16; + if (*height <= 0) + *height = 16; + } + } + // Choose default image format if specific request is unsupported if (*format != mlt_image_yuv420p && *format != mlt_image_yuv422 && *format != mlt_image_rgb @@ -259,10 +270,16 @@ static int producer_get_frame(mlt_producer producer, mlt_frame_ptr frame, int in // Set producer-specific frame properties mlt_properties_set_int(properties, "progressive", 1); mlt_profile profile = mlt_service_profile(MLT_PRODUCER_SERVICE(producer)); - mlt_properties_set_double(properties, "aspect_ratio", mlt_profile_sar(profile)); - mlt_properties_set_int(properties, "meta.media.width", profile->width); - mlt_properties_set_int(properties, "meta.media.height", profile->height); + double sar = profile ? mlt_profile_sar(profile) : 1.0; + int pw = profile ? profile->width : 16; + int ph = profile ? profile->height : 16; + mlt_properties_set_double(properties, "aspect_ratio", sar); + mlt_properties_set_int(properties, "meta.media.width", pw); + mlt_properties_set_int(properties, "meta.media.height", ph); + + + // colour is an alias for resource if (mlt_properties_get(producer_props, "colour") != NULL) mlt_properties_set(producer_props, diff --git a/src/modules/gdk/producer_pango.c b/src/modules/gdk/producer_pango.c index 650c81c43..dffabe9ad 100644 --- a/src/modules/gdk/producer_pango.c +++ b/src/modules/gdk/producer_pango.c @@ -187,6 +187,7 @@ static int parse_style(char *style) } static PangoFT2FontMap *fontmap = NULL; +static int pango_instance_count = 0; static void on_fontmap_reload(); mlt_producer producer_pango_init(const char *filename) @@ -198,6 +199,7 @@ mlt_producer producer_pango_init(const char *filename) pthread_mutex_lock(&pango_mutex); if (fontmap == NULL) fontmap = (PangoFT2FontMap *) pango_ft2_font_map_new(); + pango_instance_count++; pthread_mutex_unlock(&pango_mutex); producer->get_frame = producer_get_frame; @@ -807,6 +809,21 @@ static void producer_close(mlt_producer parent) free(self->text); free(self->font); free(self->family); + + // If this is the last instance, shutdown Pango/Fontconfig resources to avoid leaks in short-lived processes + pthread_mutex_lock(&pango_mutex); + if (pango_instance_count > 0) + pango_instance_count--; + if (pango_instance_count == 0 && fontmap) { + pango_fc_font_map_shutdown(PANGO_FC_FONT_MAP(fontmap)); + g_object_unref(fontmap); + fontmap = NULL; + FcFini(); + } + pthread_mutex_unlock(&pango_mutex); + + + parent->close = NULL; mlt_producer_close(parent); free(self); diff --git a/src/modules/movit/filter_movit_convert.cpp b/src/modules/movit/filter_movit_convert.cpp index 4189ca24c..f4611933c 100644 --- a/src/modules/movit/filter_movit_convert.cpp +++ b/src/modules/movit/filter_movit_convert.cpp @@ -759,11 +759,11 @@ static mlt_frame process(mlt_filter filter, mlt_frame frame) // The producer may still change it during get_image. // This way we do not have to modify each producer to set a valid colorspace. mlt_properties properties = MLT_FRAME_PROPERTIES(frame); - if (mlt_properties_get_int(properties, "colorspace") <= 0) - mlt_properties_set_int(properties, - "colorspace", - mlt_service_profile(MLT_FILTER_SERVICE(filter))->colorspace); - + if (mlt_properties_get_int(properties, "colorspace") <= 0) { + mlt_profile profile = mlt_service_profile(MLT_FILTER_SERVICE(filter)); + int colorspace = profile ? profile->colorspace : mlt_colorspace_bt601; + mlt_properties_set_int(properties, "colorspace", colorspace); + } frame->convert_image = convert_image; mlt_filter cpu_csc = (mlt_filter) mlt_properties_get_data(MLT_FILTER_PROPERTIES(filter), diff --git a/src/modules/xml/producer_xml.c b/src/modules/xml/producer_xml.c index 1f40f1e26..f51abe74e 100644 --- a/src/modules/xml/producer_xml.c +++ b/src/modules/xml/producer_xml.c @@ -753,9 +753,12 @@ static void on_end_link(deserialise_context context, const xmlChar *name) mlt_log_error(NULL, "[producer_xml] Invalid top of stack on link close\n"); } - if (service) { + if (service && type == mlt_dummy_producer_type) { mlt_service_close(service); free(service); + } else if (service && type != mlt_dummy_producer_type) { + // Not our dummy wrapper; restore stack to avoid freeing real services. + context_push_service(context, service, type); } } @@ -781,6 +784,22 @@ static void on_end_producer(deserialise_context context, const xmlChar *name) mlt_service service = context_pop_service(context, &type); mlt_properties properties = MLT_SERVICE_PROPERTIES(service); + // If the top of stack is a real producer (due to earlier push), consume it, + // then consume and destroy the dummy wrapper underneath. + if (service != NULL && type == mlt_producer_type) { + enum service_type dummy_type = mlt_invalid_type; + mlt_service dummy = context_pop_service(context, &dummy_type); + if (dummy && dummy_type == mlt_dummy_producer_type) { + mlt_service_close(dummy); + free(dummy); + } else if (dummy) { + // Unexpected non-dummy beneath a producer; restore it. + context_push_service(context, dummy, dummy_type); + } + // Producer end tag consumed; do not push producer back. + return; + } + if (service != NULL && type == mlt_dummy_producer_type) { mlt_service producer = NULL;