From patchwork Thu Aug 13 03:43:17 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 21622 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 A2C0B44BCD9 for ; Thu, 13 Aug 2020 06:43:33 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 7C8C468B4C4; Thu, 13 Aug 2020 06:43:33 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id D6CA36881AF for ; Thu, 13 Aug 2020 06:43:26 +0300 (EEST) Received: by mail-wm1-f65.google.com with SMTP id g75so3763925wme.4 for ; Wed, 12 Aug 2020 20:43:26 -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:mime-version :content-transfer-encoding; bh=2w2TL/OTilvO9VNAKNcjPDQkyy4JSWbox8XRjRg4mm0=; b=EczYfSWfHP0ZikLPTu5/82MVNLjrbLP8mU5bGtJ+J2FPeSbFyJNzTO4iTYLWExjQCv nW8o748Ixs7Oi0t8DLyZTSqaFwqa6jCtClkIH9cnB0qJMFxNa3S/VoTKZycn9qOiVKvn xHXhPPX6T3z//m8hoJYAblLHqIHGAcMOd7ZgT7nGu1gBPvc2EmqKcctXbfT/97CwajUo TbE+ScAw59QUTKNj1dSQRMwgvscHDU5QEbUZwUn1efrVacO9FbtnzLXmvNaGHjRSzSWq B8nygnEW405sOeJSCGKGyR6sQo+BWYj0nY0XkcXdD2kuxcUtSKgRVYEhKrcmMippOHm6 vnbw== 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:mime-version :content-transfer-encoding; bh=2w2TL/OTilvO9VNAKNcjPDQkyy4JSWbox8XRjRg4mm0=; b=tgNcKdkVJ3ZXy8NuBW43PWZZP5bom133iq31Cn2EIPH+JW/UJ+jJjUwAf9MTOP6pN6 kAGmBEtmdmnr3RFIEFRV6k4fwHyxDnq3lC5xjL6Yk7ShPLyBodKcx03ew0PwakZf+p1V 3nwQg2oqIVV6x5EIRy2Tputxyli+0N+RfmV0oKnvsl4PSNzj4E5EqM7FqJWwzcvqjExJ YhY3ix8SidTh8sH7iI9TwosVUGqZH3Ls0xa63OMJtC77QC2IEmasO2fKM+KortBRgtn7 BTtjdF5Gi0DyilXqh7VIcNLblSd7xoGJXdV6Frx0R71zrOf/FPW1dizk2nR6lwX24TXv eOBQ== X-Gm-Message-State: AOAM531qJbxeZRJshOGmUdIpuK3WgzHjmtJKv6AGQtyXo4Ie3Y+NRfgO hRvvWJdJzKN/zLt4lbKRB3+evSlJ X-Google-Smtp-Source: ABdhPJxRgJvEMLpNwe9V4Q8qDwxQCLlIrMmi1JV8ZOwCRYjJQM2ddUoUFGVp1RPMS7aa869ywroEMA== X-Received: by 2002:a1c:9616:: with SMTP id y22mr2357424wmd.100.1597290205930; Wed, 12 Aug 2020 20:43:25 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc10296.dynamic.kabel-deutschland.de. [188.193.2.150]) by smtp.gmail.com with ESMTPSA id y2sm7427945wmg.25.2020.08.12.20.43.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 12 Aug 2020 20:43:25 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Thu, 13 Aug 2020 05:43:17 +0200 Message-Id: <20200813034317.6782-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH] avfilter/formats: Fix heap-buffer overflow when merging channel layouts 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 channel layouts accepted by ff_merge_channel_layouts() are of two types: Ordinary channel layouts and generic channel layouts. These are layouts that match all layouts with a certain number of channels. Therefore parsing these channel layouts is not done in one go; instead first the intersection of the ordinary layouts of the first input list of channel layouts with the ordinary layouts of the second list is determined, then the intersection of the ordinary layouts of the first one and the generic layouts of the second one etc. In order to mark the ordinary channel layouts that have already been matched as used they are zeroed. The inner loop that does this is as follows: for (j = 0; j < b->nb_channel_layouts; j++) { if (a->channel_layouts[i] == b->channel_layouts[j]) { ret->channel_layouts[ret_nb++] = a->channel_layouts[i]; a->channel_layouts[i] = b->channel_layouts[j] = 0; } } (Here ret->channel_layouts is the array containing the intersection of the two input arrays.) Yet the problem with this code is that after a match has been found, the loop continues the search with the new value a->channel_layouts[i]. The intention of zeroing these elements was to make sure that elements already paired at this stage are ignored later. And while they are indeed ignored when pairing ordinary and generic channel layouts later, it has the exact opposite effect when pairing ordinary channel layouts. To see this consider the channel layouts A B C D E and E D C B A. In the first round, A and A will be paired and added to ret->channel_layouts. In the second round, the input arrays are 0 B C D E and E D C B 0. At first B and B will be matched and zeroed, but after doing so matching continues, but this time it will search for 0, which will match with the last entry of the second array. ret->channel_layouts now contains A B 0. In the third round, C 0 0 will be added to ret->channel_layouts etc. This gives a quadratic amount of elements, yet the amount of elements allocated for said array is only the sum of the sizes of a and b. This issue can e.g. be reproduced by ffmpeg -f lavfi -i anullsrc=cl=7.1 \ -af 'aformat=cl=mono|stereo|2.1|3.0|4.0,aformat=cl=4.0|3.0|2.1|stereo|mono' \ -f null - The fix is easy: break out of the inner loop after having found a match. Signed-off-by: Andreas Rheinhardt --- 1. This patch is intended to be applied before https://ffmpeg.org/pipermail/ffmpeg-devel/2020-August/267790.html 2. After this patch the code will stay within the buffer provided that no list contains one and the same generic channel layout multiple times (ordinary channel layouts may appear arbitrary often and there may even be ordinary channel layouts and generic channel layouts of the same channel count in the same list). But is this actually ensured? (Given that the concept of generic channel layouts is not part of the public API, but just valid in AVFilterChannelLayouts the question can be rephrased as: Is it possible for the (API) user to somehow create generic channel layouts?) libavfilter/formats.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libavfilter/formats.c b/libavfilter/formats.c index 00d050e439..f39396ed81 100644 --- a/libavfilter/formats.c +++ b/libavfilter/formats.c @@ -219,6 +219,7 @@ AVFilterChannelLayouts *ff_merge_channel_layouts(AVFilterChannelLayouts *a, if (a->channel_layouts[i] == b->channel_layouts[j]) { ret->channel_layouts[ret_nb++] = a->channel_layouts[i]; a->channel_layouts[i] = b->channel_layouts[j] = 0; + break; } } }