From 65be8cdc4dcdf95085d00afc38aef24967ff48a9 Mon Sep 17 00:00:00 2001 From: Andreas Rheinhardt Date: Fri, 7 Aug 2020 15:04:41 +0200 Subject: [PATCH] avfilter/vf_alphamerge: Fix double-free of AVFilterFormats on error The query_formats function of the alphamerge filter tries to allocate two lists of formats which on success are attached to more permanent objects (AVFilterLinks) for storage afterwards. If attaching a list to an AVFilterLink succeeds, the link becomes one of the owners of the list. Yet if attaching a list to one of its links succeeds and an error happens lateron, both lists were manually freed, which is wrong if the list is already owned by one or more links; these links' pointers to their lists will become dangling and there will be a double-free/use- after-free when these links are cleaned up automatically. This commit fixes this by removing the custom freeing code; this will temporarily add a leaking codepath (if attaching a list not already owned by a link to a link fails, the list will leak), but this will be fixed soon by making sure that an AVFilterFormats without owner will be automatically freed when attaching it to an AVFilterLink fails. At most one list leaks because as of this commit a new list is only allocated after the old list has been successfully attached to a link. Reviewed-by: Nicolas George Signed-off-by: Andreas Rheinhardt (cherry picked from commit fd1a2a54a478462212b25753e7106c13af1e33c6) --- libavfilter/vf_alphamerge.c | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/libavfilter/vf_alphamerge.c b/libavfilter/vf_alphamerge.c index 85b6d9b61a..a509f10103 100644 --- a/libavfilter/vf_alphamerge.c +++ b/libavfilter/vf_alphamerge.c @@ -55,27 +55,15 @@ static int query_formats(AVFilterContext *ctx) AV_PIX_FMT_NONE }; static const enum AVPixelFormat alpha_fmts[] = { AV_PIX_FMT_GRAY8, AV_PIX_FMT_NONE }; - AVFilterFormats *main_formats = NULL, *alpha_formats = NULL; + AVFilterFormats *main_formats = ff_make_format_list(main_fmts); int ret; - if (!(main_formats = ff_make_format_list(main_fmts)) || - !(alpha_formats = ff_make_format_list(alpha_fmts))) { - ret = AVERROR(ENOMEM); - goto fail; - } - if ((ret = ff_formats_ref(main_formats , &ctx->inputs[0]->out_formats)) < 0 || - (ret = ff_formats_ref(alpha_formats, &ctx->inputs[1]->out_formats)) < 0 || - (ret = ff_formats_ref(main_formats , &ctx->outputs[0]->in_formats)) < 0) - goto fail; - return 0; -fail: - if (main_formats) - av_freep(&main_formats->formats); - av_freep(&main_formats); - if (alpha_formats) - av_freep(&alpha_formats->formats); - av_freep(&alpha_formats); - return ret; + if ((ret = ff_formats_ref(main_formats, &ctx->inputs[0]->out_formats)) < 0 || + (ret = ff_formats_ref(main_formats, &ctx->outputs[0]->in_formats)) < 0) + return ret; + + return ff_formats_ref(ff_make_format_list(alpha_fmts), + &ctx->inputs[1]->out_formats); } static int config_input_main(AVFilterLink *inlink)