From patchwork Thu Apr 23 03:07:37 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 19189 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 CDBED44BA43 for ; Thu, 23 Apr 2020 06:08:26 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id BF11F68BE40; Thu, 23 Apr 2020 06:08:26 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f67.google.com (mail-wm1-f67.google.com [209.85.128.67]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 6105468BE0E for ; Thu, 23 Apr 2020 06:08:14 +0300 (EEST) Received: by mail-wm1-f67.google.com with SMTP id 188so4771399wmc.2 for ; Wed, 22 Apr 2020 20:08:14 -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=SChYUNhYjJfZamLzA17hbsWIfYmncgimpXeN6R3U1c4=; b=DJ550QYfl8/YMjyq+sQimDyzW6EGlJ8048NnlkvHXm/LNePX8H888XTZY7o1wjvEZj VXU/pi5unK29hZNGcViNanALocHjevaQHIpy/rXJLDY5Yb8rDZKo/w4EBaxf/GXsJWva QjdUsofUqb2l8esFhHZiuWLJJZWcKmOcSyyTdZQXW34z9F2lIRjpldZ3547gGNZN/bWI lnUeaCFzuVTA6zkNxZQ38N25ILeZxyHD6xXLHLxdq5Pqp539sfyP+kl1sifaNDDJCwg9 mhSUQXVVAGaVleygnLRhiWmoaJTy7hHY7Ard0+VYm20DO7L/I6rg8nYc4rVTm+anAZBY Itbw== 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=SChYUNhYjJfZamLzA17hbsWIfYmncgimpXeN6R3U1c4=; b=daYpglW1boDPkQeCFYVDRY5+IUn9uuQ8hApzJVx57zzgXRRhG99JN3u+k6bQAW9p9F 29dOi9Co6YbmTe1Yx6ejpQWK5011nBRIzioRdXDziwQ0PmtsLcUxOGcCggltO41n7gac GNKITDwR/zAaAL7xaVpAqwDL7b2CEep2OsfnNz0xkvFsgFQdfNZGT6Tv6GaSrsbQAy+c Kd30NDK2QRfxD5dCt8RmZWgTq9YzSu415qtUf+TwGDMGIEkOc3EZ7iJ0dSF3AYfN4qNR APGpBtZ28YJ7tAK3Xj3YJTJ1XWBqhMX9BOAivR57lnmx7flv9QsZuKd+yKLtra8tUAC7 BPqg== X-Gm-Message-State: AGi0PuZcDeNi8g4/7cUAK4ZNMGhtLGvu8W7a8exkbiiTh9d6y+GOaYFn YK4V1bPiS92FzcyW++HxFynciwfr X-Google-Smtp-Source: APiQypKvZveG0EMOEKnpYNS2NNF/ktaIz+Rj8kLjAiA42HEWpS6WHGndZp5LGt6PNX6ZXPu7ljkcOA== X-Received: by 2002:a1c:9852:: with SMTP id a79mr1587067wme.27.1587611293501; Wed, 22 Apr 2020 20:08:13 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1ab57.dynamic.kabel-deutschland.de. [188.193.171.87]) by smtp.gmail.com with ESMTPSA id m1sm1497733wro.64.2020.04.22.20.08.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Apr 2020 20:08:12 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Thu, 23 Apr 2020 05:07:37 +0200 Message-Id: <20200423030741.12158-7-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200423030741.12158-1-andreas.rheinhardt@gmail.com> References: <20200423030741.12158-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 07/11] avformat/matroskadec: Don't discard valid packets 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 Block (meaning both a Block in a BlockGroup as well as a SimpleBlock) must have at least three bytes after the field containing the encoded TrackNumber. So if there are <= 3 bytes, the Matroska demuxer would skip this block, believing it to be an empty, but valid Block. This might discard valid nonempty Blocks, namely if the track uses header stripping. And certain definitely spec-incompliant Blocks don't raise errors: Those with two or less bytes left after the encoded TrackNumber and those with three bytes left, but with flags indicating that the Block uses lacing as then there has to be further data describing the lacing. Furthermore, zero-sized packets were still possible because only the size of the last entry of a lace was checked. This commit fixes this. All spec-compliant Blocks that contain data (even if side data only) are now returned to the caller; spec-compliant Blocks that don't contain anything are not returned. Signed-off-by: Andreas Rheinhardt --- I don't know if side data-only packets can cause problems in other contexts; but they at least contain something, so they should not be ignored. libavformat/matroskadec.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c index 5643e15a20..3b1b447d8a 100644 --- a/libavformat/matroskadec.c +++ b/libavformat/matroskadec.c @@ -3020,7 +3020,9 @@ static int matroska_parse_laces(MatroskaDemuxContext *matroska, uint8_t **buf, return 0; } - av_assert0(size > 0); + if (size <= 0) + return AVERROR_INVALIDDATA; + *laces = *data + 1; data += 1; size -= 1; @@ -3046,7 +3048,7 @@ static int matroska_parse_laces(MatroskaDemuxContext *matroska, uint8_t **buf, break; } } - if (size <= total) { + if (size < total) { return AVERROR_INVALIDDATA; } @@ -3093,7 +3095,7 @@ static int matroska_parse_laces(MatroskaDemuxContext *matroska, uint8_t **buf, } data += offset; size -= offset; - if (size <= total) { + if (size < total) { return AVERROR_INVALIDDATA; } lace_size[*laces - 1] = size - total; @@ -3413,7 +3415,7 @@ static int matroska_parse_frame(MatroskaDemuxContext *matroska, { MatroskaTrackEncoding *encodings = track->encodings.elem; uint8_t *pkt_data = data; - int res; + int res = 0; AVPacket pktl, *pkt = &pktl; if (encodings && !encodings->type && encodings->scope & 1) { @@ -3449,6 +3451,9 @@ static int matroska_parse_frame(MatroskaDemuxContext *matroska, pkt_data = pr_data; } + if (!pkt_size && !additional_size) + goto no_output; + av_init_packet(pkt); if (pkt_data != data) pkt->buf = av_buffer_create(pkt_data, pkt_size + AV_INPUT_BUFFER_PADDING_SIZE, @@ -3519,6 +3524,7 @@ FF_ENABLE_DEPRECATION_WARNINGS return 0; +no_output: fail: if (pkt_data != data) av_freep(&pkt_data); @@ -3554,8 +3560,8 @@ static int matroska_parse_block(MatroskaDemuxContext *matroska, AVBufferRef *buf av_log(matroska->ctx, AV_LOG_INFO, "Invalid stream %"PRIu64"\n", num); return AVERROR_INVALIDDATA; - } else if (size <= 3) - return 0; + } else if (size < 3) + return AVERROR_INVALIDDATA; st = track->stream; if (st->discard >= AVDISCARD_ALL) return res;