From patchwork Sun Jun 14 22:36:55 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 20376 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 E15D344A534 for ; Mon, 15 Jun 2020 01:41:51 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id CB9AE68B700; Mon, 15 Jun 2020 01:41:51 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 9616768B6EA for ; Mon, 15 Jun 2020 01:41:50 +0300 (EEST) Received: by mail-wr1-f68.google.com with SMTP id x13so15179798wrv.4 for ; Sun, 14 Jun 2020 15:41:50 -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=IAp9d7PALZ/LP65PUWQ/yluwkikWgTvREntj6PEroc4=; b=M9mBVL2CrBpnsYgS/Tfxz80/fJvCRfHwVNDlBY004XgbZ1e/sFYQ5MYbsi/Ca/yT+Z rFvPWUdoneY4QKw/amK28sIgTLkMmkFUbg21/V97H1rZGP0m5SQ9ncvSGWZf5N+SaYbh C+pAOgf9jGuJLnLfDq7ldSBaPNt9NbO4u1xrJx8Qlcu/fU+ypkipevIxYCX1FfngcHXZ RkKPaYc9YkvC2RQCm9Ho/h/FqfD8cEgY5NB3hYaekX6Y8ynyISFP0VF5PIt7HLFXEox0 6emQTkBMXSHJXev0GjqwhwKK3U58FKU1TGlY4ByEz5BbQQ1Qk5DrdXJ+2tSSqnkg/qZ4 JJBA== 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=IAp9d7PALZ/LP65PUWQ/yluwkikWgTvREntj6PEroc4=; b=Ej/EebasYK1xwRsvlrfYCZCQTAoiC0NwLhnYfCgcb4sKEC1YbHEGQNCvsqcp5UeuNF gicHFQ+dflWHOga3Z79COXnLAqUEjDbM0hcEEMWr0g4rlcp9KQc9ghPO22pWd08c9Lrm sjwpKZNJ8P0ycrLvBQC7EvVIr0OW7s+eAf4xTYU0KlfZKLUJfTb+//7W8qGzW24gj9pd dBSU6ytrod1hTr19cj3Rq0J7dTTYYhXzfyF+2Tlfh53q6ElP/Cx5EYGXXBbjSgdLPw1i KT6wkRSorpe+0JH4p9H19iKI5awDhRXmTp7WSrcNXPhOVk8XvWJegG+nefSehhscz8Op Lx4A== X-Gm-Message-State: AOAM530pcNGY2UZui5EquJ7sYM8luD8ep4X1p8w/X6luEtmTRL39pP20 5LtqjnrFv+jQ0DVPMwhLjF/hi8mr X-Google-Smtp-Source: ABdhPJw7S0/bLJeg/fvBk6i7k94MPXPx5oK1EKIltoqhyECLsUexYth9mfLbFREi8XzFkM/bnU4CIA== X-Received: by 2002:adf:ff8a:: with SMTP id j10mr25408208wrr.405.1592174509844; Sun, 14 Jun 2020 15:41:49 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1ab57.dynamic.kabel-deutschland.de. [188.193.171.87]) by smtp.gmail.com with ESMTPSA id z8sm21491034wru.33.2020.06.14.15.41.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 14 Jun 2020 15:41:49 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Mon, 15 Jun 2020 00:36:55 +0200 Message-Id: <20200614223656.21338-25-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200614223656.21338-1-andreas.rheinhardt@gmail.com> References: <20200614223656.21338-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 25/26] avformat/mov: Fix memleak upon encountering repeating tags 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" mov_read_custom tries to read three strings belonging to three different tags. When an already encountered tag is encountered again, a new buffer for the string to be read is allocated and stored in the pointer destined for this particular tag. But in this scenario, said pointer already holds the address of the string read earlier, leading to a leak. This commit aborts therefore aborts the reading process upon encountering an already encountered tag. Signed-off-by: Andreas Rheinhardt --- I don't know if one should error out in this scenario or not; or one could continue (i.e. use the already existing tag) or free the old one and use the new one. But this loop is only executed three times and all three tags need to be present for them to be of any use, so simply using continue in the scenario here would not be useful. libavformat/mov.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libavformat/mov.c b/libavformat/mov.c index 47bbb3697d..a59c804d16 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -4434,6 +4434,9 @@ static int mov_read_custom(MOVContext *c, AVIOContext *pb, MOVAtom atom) } else break; + if (*p) + break; + *p = av_malloc(len + 1); if (!*p) { ret = AVERROR(ENOMEM);