From patchwork Fri May 1 00:55:20 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 19410 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 4550D44B78B for ; Fri, 1 May 2020 03:55:41 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 2B82F68C786; Fri, 1 May 2020 03:55:41 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com [209.85.128.66]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 33AAC68C5E6 for ; Fri, 1 May 2020 03:55:35 +0300 (EEST) Received: by mail-wm1-f66.google.com with SMTP id u16so4624093wmc.5 for ; Thu, 30 Apr 2020 17:55:35 -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=WYHsF6IZ7AuHoPW84biJlRHqeTtyES3kFiO9uep34mY=; b=vFxTfVkpQtc26dtoZem289Iy5jL9q70T1PNDFxcv0UcfoTizBVJL0Ar9LEZBWqOm56 xpMhzx3CuKp6BbPiK3AixJANA4MnqJLQ0VIU2xqWNiXSUpFSlFFSpF+7NVFH097XHTW8 7mBeqq9+7KgyQmI/lbfWkSiK2MI4ClrmhQuQhz/yrTuq55IgfoJJWN4wi6M0Aet+WHtF KOnuj/IaQQ92DR2olBu8T2df47JRXGWyboRa/Y0tb/AirMiAeTDevNKV1UHwHUBXNuPF bicSvH/n8rQA7jkAEp37+6K1oDUJ57NrdkFKsZxXGhE8JbgxBw8/8X8D9MF/0SPLWYcm 3/6w== 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=WYHsF6IZ7AuHoPW84biJlRHqeTtyES3kFiO9uep34mY=; b=stIjNNvJqBSQLHzMQNDxhB865kASOu3c60YBdVTQDvg9IpHAhPp2aEMb7lTOlu3DVU j+3Xf03fsdRFrC1185FW9qQvzL27OTg56wwVVRzaGb1TjK6rTogpRRwDKfp76utaxlxn sj/wGwMPBx4pNwCBZSih2O/qM2kNSkU3bwoo6lPHaEOcl+vuv3UUioImcttHH5cUfXDM 27L/9zt0Z3sob+1ufpT2q8ZPaY15Qp9i0hVA5eXp7nxTcoPbl6r1XZoCxgGw+FFinTQs xK2tMCX1Yzxzq1gdjUVSDh+pxAQIW07rhPG66lS5fOtU6AAgeLhuPGmK4qcIM7SJjsaR HCOA== X-Gm-Message-State: AGi0Pua4NRKc0RYAcv9Hkfky21RLtkGdbFiqYewQ+iBU+vES/zykViub P7mVAkIxGxWEgSFKkxeHPLpeF1I9 X-Google-Smtp-Source: APiQypLwKG4swF6NpwZKD7i8P4CK+mYGhZad1VEnK0u0K22RzM+GDEl1mDu9EmCNXFCIi62VOr+E9w== X-Received: by 2002:a1c:bd08:: with SMTP id n8mr1259856wmf.23.1588294534231; Thu, 30 Apr 2020 17:55:34 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1ab57.dynamic.kabel-deutschland.de. [188.193.171.87]) by smtp.gmail.com with ESMTPSA id k14sm2073444wrp.53.2020.04.30.17.55.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 Apr 2020 17:55:33 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Fri, 1 May 2020 02:55:20 +0200 Message-Id: <20200501005522.16402-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 1/3] avformat/matroskadec: Sanitize SeekHead entries 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" A Seek element in a Matroska SeekHead should contain a SeekID and a SeekPosition element and upon reading, they should be sanitized: Given that IDs are restricted to 32 bit, longer SeekIDs should be treated as invalid. Instead currently the lower 32 bits have been used. For SeekPosition, no checks were performed for the element to be present and if present, whether it was excessively large (i.e. the absolute file position described by it exceeding INT64_MAX). The SeekPosition element had a default value of -1 which means that a check seems to have been intended; but it was not implemented. This commit adds a check for overflow to the calculation of the absolute file position of the referenced level 1 elements. Using -1 (i.e. UINT64_MAX) as default value for SeekPosition implies that a Seek element without SeekPosition will run afoul of this check. Signed-off-by: Andreas Rheinhardt --- libavformat/matroskadec.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c index 8e1326abf6..dea8f14f9e 100644 --- a/libavformat/matroskadec.c +++ b/libavformat/matroskadec.c @@ -1865,8 +1865,12 @@ static void matroska_execute_seekhead(MatroskaDemuxContext *matroska) MatroskaSeekhead *seekheads = seekhead_list->elem; uint32_t id = seekheads[i].id; int64_t pos = seekheads[i].pos + matroska->segment_start; + MatroskaLevel1Element *elem; - MatroskaLevel1Element *elem = matroska_find_level1_elem(matroska, id); + if (id != seekheads[i].id || pos < matroska->segment_start) + continue; + + elem = matroska_find_level1_elem(matroska, id); if (!elem || elem->parsed) continue; From patchwork Fri May 1 00:55:21 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 19411 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 50AA144B81F for ; Fri, 1 May 2020 03:56:08 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 3296068C7D0; Fri, 1 May 2020 03:56:08 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f42.google.com (mail-wm1-f42.google.com [209.85.128.42]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id E184468C5E6 for ; Fri, 1 May 2020 03:56:01 +0300 (EEST) Received: by mail-wm1-f42.google.com with SMTP id h4so4339719wmb.4 for ; Thu, 30 Apr 2020 17:56:01 -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=QRiGOP8qWxIbK3BMbkjeVsB5IDynvr2afEhAxkdQsKY=; b=a7AE0EqbQ+o8x9/nkABaoeqGSCTuoPw38rSrcuzaeK9+c5XVTG04SfguH6rDK1nXtz uKtGP+vfJZIcMMVhYy/TDLoNkIxAV2srWVZyoxLdK6dK2tB5zSu9QClkUQW+tg29G/0N ZKEfDNF3fuhuTG613gGKs0r4MtoXSy8/bl2MXyPYYyJGHBc4T1V8XEjiGycN7ZaO20eh lQlJ+3+kz9XZwvhLmFtwZZvxfJfE/RyCQfS04vLXYh88Qsw2yyrIQ5upYc2l+AWIHBTn KC2DIb06gplfebZhFD1JdZG5SaRHV3o7HBMpe3z/KeLQlTJgEXJwAapyRbr0+ZgaKQRG AgyA== 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=QRiGOP8qWxIbK3BMbkjeVsB5IDynvr2afEhAxkdQsKY=; b=aM3YwFCWInKigUfDa9H4+xPcMYmEJ3OoIzE5pvIshgVvGQR2Zh0QS6CS9h3SMw4zvV eKRmVNsZA74v5Wx5x145B5FR1D0LwlkMrruxQaj28RPt5lPzzwRBNCHxVRuhN1NRZ7zz y31LGbx1YPzXcn77IAgpsTNFiBuYkMixIcOUo4d9K1U19vfBDu2Q9N/C8G8uStu6lZ3K VXhyAAeOiHKzQcQBVD8wzl468LEXDw15DDEHAVmt74RuK+puKsB7+08L2dIdN7iQ/AZs nvYEwBdZ4MNAsp5jFpMH/8pgF2UzwFgCGgR3dqI8+JZcLMOp5JXRALuhEx/52w1I6tAN yVvQ== X-Gm-Message-State: AGi0PuYVQz8R32zXMzbyS3EfPI4qB4KVUsjEI+6SzF8YEkMAiRnsjf7c K1IHX+zG9sFHIuAHHOu5YIkzJ48O X-Google-Smtp-Source: APiQypI6TH1JmZjXnzNAlewHA/dAiEyMHH6IvvDa0721sLi5wgfDYNcSUliAJMYm2hpPlRMdqw5K7w== X-Received: by 2002:a7b:c0d5:: with SMTP id s21mr1255428wmh.107.1588294560788; Thu, 30 Apr 2020 17:56:00 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1ab57.dynamic.kabel-deutschland.de. [188.193.171.87]) by smtp.gmail.com with ESMTPSA id k14sm2073444wrp.53.2020.04.30.17.55.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 Apr 2020 17:56:00 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Fri, 1 May 2020 02:55:21 +0200 Message-Id: <20200501005522.16402-2-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200501005522.16402-1-andreas.rheinhardt@gmail.com> References: <20200501005522.16402-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 2/3] avformat/matroskadec: Improve handling of circular SeekHeads 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" There can be more than one SeekHead in a Matroska file, but most of the other level 1 elements can only occur once.* Therefore the Matroska demuxer only allows one entry per ID in its internal list of level 1 elements known to it; the only exception to this are SeekHeads. The only exception to this are SeekHeads: When one is encountered (either directly or in the list of entries read from SeekHeads), a new entry in the list of known level-1 elements is always added, even when this entry is actually already known. This leads to lots of seeks in case of circular SeekHeads: Each time a SeekHead is parsed, a new entry for a SeekHead will be added to the list of entries read from SeekHeads. The exception for SeekHeads mentioned above now implies that this SeekHead will always appear new and unparsed and parsing will be attempted. This continued until the list of known level-1 elements is full. Fixing this is pretty simple: Don't add a new entry for a SeekHead if its position matches the position of an already known SeekHead. *: Actually, there can be multiple Tags and several other level 1 elements are "identically recurring" which means they may be resent multiple times, but each instance must be absolutely identical to the previous. Signed-off-by: Andreas Rheinhardt --- libavformat/matroskadec.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c index dea8f14f9e..25eed210c5 100644 --- a/libavformat/matroskadec.c +++ b/libavformat/matroskadec.c @@ -1134,7 +1134,7 @@ static int is_ebml_id_valid(uint32_t id) * an entry already exists, return the existing entry. */ static MatroskaLevel1Element *matroska_find_level1_elem(MatroskaDemuxContext *matroska, - uint32_t id) + uint32_t id, int64_t pos) { int i; MatroskaLevel1Element *elem; @@ -1147,18 +1147,17 @@ static MatroskaLevel1Element *matroska_find_level1_elem(MatroskaDemuxContext *ma return NULL; // There can be multiple seekheads. - if (id != MATROSKA_ID_SEEKHEAD) { - for (i = 0; i < matroska->num_level1_elems; i++) { - if (matroska->level1_elems[i].id == id) + for (i = 0; i < matroska->num_level1_elems; i++) { + if (matroska->level1_elems[i].id == id) { + if (matroska->level1_elems[i].pos == pos || + id != MATROSKA_ID_SEEKHEAD) return &matroska->level1_elems[i]; } } // Only a completely broken file would have more elements. - // It also provides a low-effort way to escape from circular seekheads - // (every iteration will add a level1 entry). if (matroska->num_level1_elems >= FF_ARRAY_ELEMS(matroska->level1_elems)) { - av_log(matroska->ctx, AV_LOG_ERROR, "Too many level1 elements or circular seekheads.\n"); + av_log(matroska->ctx, AV_LOG_ERROR, "Too many level1 elements.\n"); return NULL; } @@ -1407,7 +1406,7 @@ static int ebml_parse(MatroskaDemuxContext *matroska, if (id == MATROSKA_ID_CUES) matroska->cues_parsing_deferred = 0; if (syntax->type == EBML_LEVEL1 && - (level1_elem = matroska_find_level1_elem(matroska, syntax->id))) { + (level1_elem = matroska_find_level1_elem(matroska, syntax->id, pos))) { if (!level1_elem->pos) { // Zero is not a valid position for a level 1 element. level1_elem->pos = pos; @@ -1870,7 +1869,7 @@ static void matroska_execute_seekhead(MatroskaDemuxContext *matroska) if (id != seekheads[i].id || pos < matroska->segment_start) continue; - elem = matroska_find_level1_elem(matroska, id); + elem = matroska_find_level1_elem(matroska, id, pos); if (!elem || elem->parsed) continue; From patchwork Fri May 1 00:55:22 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 19412 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 55C7E44B81F for ; Fri, 1 May 2020 03:56:10 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 3EBC068C7D7; Fri, 1 May 2020 03:56:10 +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 1FE5068C7BA for ; Fri, 1 May 2020 03:56:04 +0300 (EEST) Received: by mail-wr1-f67.google.com with SMTP id x18so9776758wrq.2 for ; Thu, 30 Apr 2020 17:56:04 -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=w6/T8SWevktQIHb7KuCyvp7RNEUuPfvd8etzgq8swJo=; b=tlMo1m+egecnhPTxHOru2h5Lwk8bztBpJVrDpqD1Htyfp7i+/YywAwAGVWtyJfjDSa Qhxzig4x2IMfX7Dkf9gTk/HhNpTpYBuSROpiSlzELmAsjrm7Z4XB2WIqP8uQFNWN3682 ZwFy6vxnzxB42zNu5iQP/B6CY3cYWxWVvHvXRZU9ksKddEsLEVYmaE14wgzlp4487m4u f7gBEkzZFueEtv5qP+xRPl0tqHT0PFQU0G4K0KVa1uOrr8F0T3CvAZgvNTjJo0reMZQm L5Pir9ZOZEtQkcZt81Hg9KfUgQNFKnHvncBH0FZgT6NVxEXkaRoYiJ4+Nz//GGXjD0sX ij3g== 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=w6/T8SWevktQIHb7KuCyvp7RNEUuPfvd8etzgq8swJo=; b=KVL4fsU6j/l3JKib4Fph02jdL+VuHit1ue0kx+1/48oKCmtczSuyYVPCEI4IHrqghX CH0vPwB97/50PTWArRGCN9dPyltFF3bh3yRmny4+MIMJgEajktHHiQ4wQl1PQwAxDARU uSmRf2AyoQjQrug9hQ8u0v8MV74ST8LdRaReDIBmSyzbhhx57QspW+rdzdAcorFClbn/ x2/sGIobNN6C0NfRy9xGlvJygcG/SfIvLirE0LDm4xtpZZwFg3vqHrA9/dUEFRpsRDZM vViEZhSSdAkYQ07YIDHBbqEdO33cFkY0F1Xn7eDP1HPTJBSBtbja6juRtlRLfeGgFVv4 cRRA== X-Gm-Message-State: AGi0PuZKoX8SmAjIbeQyK+91E8y2dutbQkd6HWHbzyaD3bABBZFT1cEk S4B2HHIOoSwgePEnrS8FeeQZ0jVL X-Google-Smtp-Source: APiQypKa6gcDhapy5nsbbAOKOGTOjCsdCWXOjPfFlGQFTf8/69ncZ5l98qJkW7gZsnK7YQEHbxuBNw== X-Received: by 2002:a05:6000:11cb:: with SMTP id i11mr1319917wrx.339.1588294563320; Thu, 30 Apr 2020 17:56:03 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1ab57.dynamic.kabel-deutschland.de. [188.193.171.87]) by smtp.gmail.com with ESMTPSA id k14sm2073444wrp.53.2020.04.30.17.56.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 Apr 2020 17:56:02 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Fri, 1 May 2020 02:55:22 +0200 Message-Id: <20200501005522.16402-3-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200501005522.16402-1-andreas.rheinhardt@gmail.com> References: <20200501005522.16402-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 3/3] avformat/matroskadec: Allow multiple Tags elements 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 Matroska specification allows multiple (level 1) Tags elements per file, yet our demuxer didn't: While it parsed any amount of Tags elements it found in front of the Clusters (albeit with warnings because of duplicate elements), it would treat any Tags element only referenced via a SeekHead entry as already parsed if any Tags element has already been parsed; therefore this Tags element would not be parsed at all. Signed-off-by: Andreas Rheinhardt --- libavformat/matroskadec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c index 25eed210c5..aede93d994 100644 --- a/libavformat/matroskadec.c +++ b/libavformat/matroskadec.c @@ -1146,11 +1146,11 @@ static MatroskaLevel1Element *matroska_find_level1_elem(MatroskaDemuxContext *ma if (id == MATROSKA_ID_CLUSTER) return NULL; - // There can be multiple seekheads. + // There can be multiple SeekHeads and Tags. for (i = 0; i < matroska->num_level1_elems; i++) { if (matroska->level1_elems[i].id == id) { if (matroska->level1_elems[i].pos == pos || - id != MATROSKA_ID_SEEKHEAD) + id != MATROSKA_ID_SEEKHEAD && id != MATROSKA_ID_TAGS) return &matroska->level1_elems[i]; } }