From patchwork Sat Oct 17 18:22:39 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 23040 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 3CD0844B29E for ; Sat, 17 Oct 2020 21:23:21 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 2B28268B9EB; Sat, 17 Oct 2020 21:23:21 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 33D7768B981 for ; Sat, 17 Oct 2020 21:23:15 +0300 (EEST) Received: by mail-wr1-f67.google.com with SMTP id h5so7000421wrv.7 for ; Sat, 17 Oct 2020 11:23:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=S4KpIWzMqOzmGlqzXYffBEll+/Fi0VA9jvD9CTIF6JU=; b=siAWQUZcICjkqd+hHJFKt36M3DSfmLxS5d5v5WIPeIr36m03Ttt1naVE/HwtqPIxKS 8J0OTAIo3sJbMoojsIhPFfZ9G6pdv8TKCLwr5C7UjN4AvMkrVwfiOIXFcO5TQrGi2AXT P65m34bOoarcoe/3mOFRoWxX3tOXUbaHro9cY4LnNjp7Joofj+ahw/s4jnb5h+uzWdXl nzDsAWH0MkhyYCZtFsIzaS41rHVqiftZzSg2xnIzUjOSBr4fmZ+HhXftIIQ4IhuH/6sN +UMoJ6vYglLCbIhCA2VV5CwXzzqMMF68MqnKjHs+3aK5fUooxVrSPCUj11XL+S5ffKp4 a2mA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=S4KpIWzMqOzmGlqzXYffBEll+/Fi0VA9jvD9CTIF6JU=; b=gtNHqaUjuGmjTQyQNb8BVrprDu9BLJOPWnGRyOis6Y88xu0xXHUVUKneUKBBE3s5ot FE48dKwzG/zQrBMCK/RlzMTsOdBDUsCCMQ2ZbdYfhbjkhmDCZl3IhGS8EC8uysjOyE4V YW0NIwMXD8S49jRQgssdoXnb3wonLXMx9I4WWL4UGy3eSLXe/atySY3Ud5gAwF3o69jR RzDxzJqxOdodICxFGbYUXd/ZIRI0Y2XZrrXXRO3A5JyEq0nXT3eMFyDyM/soMxs5qvTN h5qlXgMEkhI9ZoSrTZtEGIiU3BrgXyiPTaMbCcpoAob+uNfz0J4TipYr1ifLnfdrbJRs Fhuw== X-Gm-Message-State: AOAM530ga+0k4LyTOptgSzbWc1fQAzxsnV0AnM+SKptMTKx54GPtXs/4 IEA1zX5cQu+MfvQyhM4FYUDaUNPpR9Q= X-Google-Smtp-Source: ABdhPJwXwCX9DeAIvAli7N4E2jcyGO8X9+7RB3Vmnn6Er2pI0iEKQz3vde1xwFjvVtaptm5+DR4oog== X-Received: by 2002:a5d:6642:: with SMTP id f2mr11535912wrw.374.1602958994277; Sat, 17 Oct 2020 11:23:14 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1aa4b.dynamic.kabel-deutschland.de. [188.193.170.75]) by smtp.gmail.com with ESMTPSA id d20sm10440688wra.38.2020.10.17.11.23.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 17 Oct 2020 11:23:13 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Sat, 17 Oct 2020 20:22:39 +0200 Message-Id: <20201017182248.577108-2-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20201017182248.577108-1-andreas.rheinhardt@gmail.com> References: <20201017182248.577108-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 02/11] avcodec/movtextdec: Fix leaks of strings upon reallocation failure X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Andreas Rheinhardt Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Up until now, the 3GPP Timed Text decoder used av_dynarray_add() for a list of font entries, a structure which contains an allocated string. The font entries are owned by the pointers in the dynamic array and are therefore unsuitable for av_dynarray_add() which simply frees the array, but not the font entries and of course not the strings. The latter all leak if reallocating the dynamic array fails. This commit fixes this. It stops reallocating the array altogether: After all, the final number of elements (pending errors) is already known in advance. Furthermore, the font entries are now the entries of the new array, i.e. the font entries are no longer allocated separately. This also removes one level of indirection. Signed-off-by: Andreas Rheinhardt --- libavcodec/movtextdec.c | 61 ++++++++++++++++------------------------- 1 file changed, 24 insertions(+), 37 deletions(-) diff --git a/libavcodec/movtextdec.c b/libavcodec/movtextdec.c index 068bdb7802..ad60c77519 100644 --- a/libavcodec/movtextdec.c +++ b/libavcodec/movtextdec.c @@ -102,15 +102,14 @@ typedef struct { StyleBox *s_temp; HighlightBox h; HilightcolorBox c; - FontRecord **ftab; - FontRecord *ftab_temp; + FontRecord *ftab; TextWrapBox w; MovTextDefault d; uint8_t box_flags; uint16_t style_entries, ftab_entries; uint64_t tracksize; int size_var; - int count_s, count_f; + int count_s; int readorder; int frame_width; int frame_height; @@ -137,16 +136,8 @@ static void mov_text_cleanup(MovTextContext *m) static void mov_text_cleanup_ftab(MovTextContext *m) { - int i; - if (m->ftab_temp) - av_freep(&m->ftab_temp->font); - av_freep(&m->ftab_temp); - if (m->ftab) { - for(i = 0; i < m->count_f; i++) { - av_freep(&m->ftab[i]->font); - av_freep(&m->ftab[i]); - } - } + for (unsigned i = 0; i < m->ftab_entries; i++) + av_freep(&m->ftab[i].font); av_freep(&m->ftab); m->ftab_entries = 0; } @@ -156,9 +147,9 @@ static int mov_text_tx3g(AVCodecContext *avctx, MovTextContext *m) uint8_t *tx3g_ptr = avctx->extradata; int i, box_size, font_length; int8_t v_align, h_align; + unsigned ftab_entries; StyleBox s_default; - m->count_f = 0; m->ftab_entries = 0; box_size = BOX_SIZE_INITIAL; /* Size till ftab_entries */ if (avctx->extradata_size < box_size) @@ -223,7 +214,16 @@ static int mov_text_tx3g(AVCodecContext *avctx, MovTextContext *m) // ftab tx3g_ptr += 4; - m->ftab_entries = AV_RB16(tx3g_ptr); + // In case of broken header, init default font + m->d.font = ASS_DEFAULT_FONT; + + ftab_entries = AV_RB16(tx3g_ptr); + if (!ftab_entries) + return 0; + m->ftab = av_calloc(ftab_entries, sizeof(*m->ftab)); + if (!m->ftab) + return AVERROR(ENOMEM); + m->ftab_entries = ftab_entries; tx3g_ptr += 2; for (i = 0; i < m->ftab_entries; i++) { @@ -233,12 +233,7 @@ static int mov_text_tx3g(AVCodecContext *avctx, MovTextContext *m) mov_text_cleanup_ftab(m); return -1; } - m->ftab_temp = av_mallocz(sizeof(*m->ftab_temp)); - if (!m->ftab_temp) { - mov_text_cleanup_ftab(m); - return AVERROR(ENOMEM); - } - m->ftab_temp->fontID = AV_RB16(tx3g_ptr); + m->ftab[i].fontID = AV_RB16(tx3g_ptr); tx3g_ptr += 2; font_length = *tx3g_ptr++; @@ -247,26 +242,18 @@ static int mov_text_tx3g(AVCodecContext *avctx, MovTextContext *m) mov_text_cleanup_ftab(m); return -1; } - m->ftab_temp->font = av_malloc(font_length + 1); - if (!m->ftab_temp->font) { + m->ftab[i].font = av_malloc(font_length + 1); + if (!m->ftab[i].font) { mov_text_cleanup_ftab(m); return AVERROR(ENOMEM); } - memcpy(m->ftab_temp->font, tx3g_ptr, font_length); - m->ftab_temp->font[font_length] = '\0'; - av_dynarray_add(&m->ftab, &m->count_f, m->ftab_temp); - if (!m->ftab) { - mov_text_cleanup_ftab(m); - return AVERROR(ENOMEM); - } - m->ftab_temp = NULL; + memcpy(m->ftab[i].font, tx3g_ptr, font_length); + m->ftab[i].font[font_length] = '\0'; tx3g_ptr = tx3g_ptr + font_length; } - // In case of broken header, init default font - m->d.font = ASS_DEFAULT_FONT; for (i = 0; i < m->ftab_entries; i++) { - if (m->d.fontID == m->ftab[i]->fontID) - m->d.font = m->ftab[i]->font; + if (m->d.fontID == m->ftab[i].fontID) + m->d.font = m->ftab[i].font; } return 0; } @@ -405,8 +392,8 @@ static int text_to_ass(AVBPrint *buf, const char *text, const char *text_end, av_bprintf(buf, "{\\fs%d}", m->s[entry]->fontsize); if (m->s[entry]->style_fontID != m->d.fontID) for (i = 0; i < m->ftab_entries; i++) { - if (m->s[entry]->style_fontID == m->ftab[i]->fontID) - av_bprintf(buf, "{\\fn%s}", m->ftab[i]->font); + if (m->s[entry]->style_fontID == m->ftab[i].fontID) + av_bprintf(buf, "{\\fn%s}", m->ftab[i].font); } if (m->d.color != m->s[entry]->color) { color = m->s[entry]->color;