From patchwork Sun Sep 20 14:37:28 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 22529 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 64BD944B734 for ; Sun, 20 Sep 2020 17:37:49 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 4094268B675; Sun, 20 Sep 2020 17:37:49 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f43.google.com (mail-wm1-f43.google.com [209.85.128.43]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 5228F680310 for ; Sun, 20 Sep 2020 17:37:42 +0300 (EEST) Received: by mail-wm1-f43.google.com with SMTP id a9so10053625wmm.2 for ; Sun, 20 Sep 2020 07:37:42 -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:reply-to:mime-version :content-transfer-encoding; bh=0P2URrqrSBEwm7EtUb3SgH1NLxGaSqe9G8FUbZ9phBA=; b=sGIuEulHXbZFCCP0RWBfCdMIwfsSra0eVltn1/Wvp85T7ZcealyBSZm84TwGalnCRx FeNVp7DrJ028kKh8NrjDPnL+CTQ/jzJtjVTjseiDo7iZ8dnHKvpqA4jxKAARoYEjJkZ6 MTfL/gRq2Xw1Wtqch+zSCATCAU74BAkmZBSQ/naiVnrxzS6jHSE8vz8iDUV1abNDj91H +IWUN7tGR1IuzcQrXrSczSnDB9G5p3/3FMJWmFV4ZWGZQR2kaYNdPa4aR8xW2mdF+WU8 rDaQOSH2/o0vDYYcqnC0qzj9ERH77xi4L8tzqL6JmQLe7KLL+KFXm/Fu4eJ5mgxLdC2X gYuA== 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:reply-to :mime-version:content-transfer-encoding; bh=0P2URrqrSBEwm7EtUb3SgH1NLxGaSqe9G8FUbZ9phBA=; b=iamNad2A3S7LrTC1FW0tvsHCJm8xyOO1ZX44NBiq0UGEgCpUMqKLwK0tbEQdBpUDH9 xgzIMarkWtUhIZFzYByzjLRESkA1u+SJJp9x+G9p2o2UgVUGMmmyqVpsbxk5Y3fDHUC8 BxtgxGs2RRlJhFenpISlfIpVHOwTAvnlO4WiYnjwukw7S2XR5q9BudHbvgVmsSg7uD/q +MjyC1U1IucuPfInXYN/tyJCYW34LFcbq1mlW+XxKojcA1o1P+38sWPKU3tzWq7ro0PP sLV2u/07K5QyR+YdzlsT3BVCxWdjJkMgXpsLLw2E+fAKQvw5XdUK+nooAjssIMSYWogN nnvw== X-Gm-Message-State: AOAM531dhsVI9Qv6EXrQtvIhSOz4Dv+XuGWfLrI9IoYDzEmndr45qogL kNeTubXaWKBcJcv/pzfUdSqRus7i/hQ= X-Google-Smtp-Source: ABdhPJw1aKNsv5sVtc0UhXh1AKG4a96elilAul9eW28A/gkokIIX34sMEj7ALgmmnpHS9sxmgx4lgQ== X-Received: by 2002:a7b:cc09:: with SMTP id f9mr7978483wmh.93.1600612661328; Sun, 20 Sep 2020 07:37:41 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1fb0f.dynamic.kabel-deutschland.de. [188.193.251.15]) by smtp.gmail.com with ESMTPSA id 2sm14288528wmf.25.2020.09.20.07.37.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 20 Sep 2020 07:37:40 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Sun, 20 Sep 2020 16:37:28 +0200 Message-Id: <20200920143728.1896746-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH] avformat/tedcaptionsdec: Fix leak of AVBPrint upon error 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" The tedcaptions demuxer uses an AVBPrint whose string is not restricted to its internal buffer; it therefore needs to be cleaned up, yet this is not done on error, as parse_file() returned simply returned directly. This is fixed by going to fail first in such cases. Furthermore, there is also a second way how this string can leak: By having more than one subtitle per subtitle block, as the new one simply overwrites the old one in this case as the AVBPrint is initialized each time upon encountering a subtitle line. The code has been modified to simply append the new subtitle to the old one, so that the old one can't leak any more. Signed-off-by: Andreas Rheinhardt --- libavformat/tedcaptionsdec.c | 73 ++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 37 deletions(-) diff --git a/libavformat/tedcaptionsdec.c b/libavformat/tedcaptionsdec.c index 3255819e77..6c25d602d6 100644 --- a/libavformat/tedcaptionsdec.c +++ b/libavformat/tedcaptionsdec.c @@ -94,25 +94,20 @@ static int parse_string(AVIOContext *pb, int *cur_byte, AVBPrint *bp, int full) { int ret; - av_bprint_init(bp, 0, full ? AV_BPRINT_SIZE_UNLIMITED : AV_BPRINT_SIZE_AUTOMATIC); ret = expect_byte(pb, cur_byte, '"'); if (ret < 0) - goto fail; + return ret; while (*cur_byte > 0 && *cur_byte != '"') { if (*cur_byte == '\\') { next_byte(pb, cur_byte); - if (*cur_byte < 0) { - ret = AVERROR_INVALIDDATA; - goto fail; - } + if (*cur_byte < 0) + return AVERROR_INVALIDDATA; if ((*cur_byte | 32) == 'u') { unsigned chr = 0, i; for (i = 0; i < 4; i++) { next_byte(pb, cur_byte); - if (!HEX_DIGIT_TEST(*cur_byte)) { - ret = ERR_CODE(*cur_byte); - goto fail; - } + if (!HEX_DIGIT_TEST(*cur_byte)) + return ERR_CODE(*cur_byte); chr = chr * 16 + HEX_DIGIT_VAL(*cur_byte); } av_bprint_utf8(bp, chr); @@ -126,22 +121,18 @@ static int parse_string(AVIOContext *pb, int *cur_byte, AVBPrint *bp, int full) } ret = expect_byte(pb, cur_byte, '"'); if (ret < 0) - goto fail; - if (full && !av_bprint_is_complete(bp)) { - ret = AVERROR(ENOMEM); - goto fail; - } - return 0; + return ret; + if (full && !av_bprint_is_complete(bp)) + return AVERROR(ENOMEM); -fail: - av_bprint_finalize(bp, NULL); - return ret; + return 0; } static int parse_label(AVIOContext *pb, int *cur_byte, AVBPrint *bp) { int ret; + av_bprint_init(bp, 0, AV_BPRINT_SIZE_AUTOMATIC); ret = parse_string(pb, cur_byte, bp, 0); if (ret < 0) return ret; @@ -195,6 +186,8 @@ static int parse_file(AVIOContext *pb, FFDemuxSubtitlesQueue *subs) int64_t pos, start, duration; AVPacket *pkt; + av_bprint_init(&content, 0, AV_BPRINT_SIZE_UNLIMITED); + next_byte(pb, &cur_byte); ret = expect_byte(pb, &cur_byte, '{'); if (ret < 0) @@ -206,34 +199,34 @@ static int parse_file(AVIOContext *pb, FFDemuxSubtitlesQueue *subs) if (ret < 0) return AVERROR_INVALIDDATA; while (1) { - content.size = 0; start = duration = AV_NOPTS_VALUE; ret = expect_byte(pb, &cur_byte, '{'); if (ret < 0) - return ret; + goto fail; pos = avio_tell(pb) - 1; while (1) { ret = parse_label(pb, &cur_byte, &label); if (ret < 0) - return ret; + goto fail; if (!strcmp(label.str, "startOfParagraph")) { ret = parse_boolean(pb, &cur_byte, &start_of_par); if (ret < 0) - return ret; + goto fail; } else if (!strcmp(label.str, "content")) { ret = parse_string(pb, &cur_byte, &content, 1); if (ret < 0) - return ret; + goto fail; } else if (!strcmp(label.str, "startTime")) { ret = parse_int(pb, &cur_byte, &start); if (ret < 0) - return ret; + goto fail; } else if (!strcmp(label.str, "duration")) { ret = parse_int(pb, &cur_byte, &duration); if (ret < 0) - return ret; + goto fail; } else { - return AVERROR_INVALIDDATA; + ret = AVERROR_INVALIDDATA; + goto fail; } skip_spaces(pb, &cur_byte); if (cur_byte != ',') @@ -242,18 +235,22 @@ static int parse_file(AVIOContext *pb, FFDemuxSubtitlesQueue *subs) } ret = expect_byte(pb, &cur_byte, '}'); if (ret < 0) - return ret; + goto fail; if (!content.size || start == AV_NOPTS_VALUE || - duration == AV_NOPTS_VALUE) - return AVERROR_INVALIDDATA; + duration == AV_NOPTS_VALUE) { + ret = AVERROR_INVALIDDATA; + goto fail; + } pkt = ff_subtitles_queue_insert(subs, content.str, content.len, 0); - if (!pkt) - return AVERROR(ENOMEM); + if (!pkt) { + ret = AVERROR(ENOMEM); + goto fail; + } pkt->pos = pos; pkt->pts = start; pkt->duration = duration; - av_bprint_finalize(&content, NULL); + av_bprint_clear(&content); skip_spaces(pb, &cur_byte); if (cur_byte != ',') @@ -262,14 +259,16 @@ static int parse_file(AVIOContext *pb, FFDemuxSubtitlesQueue *subs) } ret = expect_byte(pb, &cur_byte, ']'); if (ret < 0) - return ret; + goto fail; ret = expect_byte(pb, &cur_byte, '}'); if (ret < 0) - return ret; + goto fail; skip_spaces(pb, &cur_byte); if (cur_byte != AVERROR_EOF) - return ERR_CODE(cur_byte); - return 0; + ret = ERR_CODE(cur_byte); +fail: + av_bprint_finalize(&content, NULL); + return ret; } static av_cold int tedcaptions_read_header(AVFormatContext *avf)