From patchwork Fri Aug 25 18:59:51 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dale Curtis X-Patchwork-Id: 4842 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.2.15.201 with SMTP id 70csp574882jao; Fri, 25 Aug 2017 12:06:50 -0700 (PDT) X-Received: by 10.28.131.81 with SMTP id f78mr201741wmd.181.1503688010291; Fri, 25 Aug 2017 12:06:50 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1503688010; cv=none; d=google.com; s=arc-20160816; b=rYHP64/Zrn+unZjggyDVwmhmXAlcMFT7SICXGhzL1oAFshu5iGN+f2khD95AGS7oKA 7GOmtpMjbPgQH4CmU6Jidac45vG2Imi3jLG/LyE5BHpmPHTnesNzLQ9pgUTg9xLNZaH5 mP9/4WjG6GJ1JpSQGAq5Y/BlzLUPLAS0oAdnSWg3KdjOl61wb2TA8oBgdtgp+XHtqPc7 8OHZQH6a7yjhSwnt3XaNj5Iwjeto4h5WpyMiXhw3GTIP4wzsTyqwYGGmIh2Yy0oPQ+KS iHqpXVd2XPBo0Jqt412kxmIC6zjsY9OKev59sTlQxwWqsxO8CI2GLY02gfUQI4dA71Dn +daQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:subject:to :message-id:date:from:references:in-reply-to:mime-version :dkim-signature:dkim-signature:delivered-to :arc-authentication-results; bh=KDO11/q8I3icqQB5UhShvg1FEkSnAa0QXCT/mL48zDs=; b=RiQ9VVwc3+z0nh8yAzFaRWmc0HeoQ6SBDseAbSXxq4nNgmBj9YrBGQJrdE3wTS3e54 Ii2EFiIHgeRTEHM06zo11xjLe6jA2NCzt256r4zNaeyk6OyzyHN1X4w9pRAkqd63HsJD k4QFazK8/Omvr6BlEYs71RJjokF/7+5uKFiO0FBtwzslqwyaVDXS5eGjgRTBQjy388Ad +umbeAG7flJ0gjnweBquFxGbE/hfbsB77iN3JUqmkAxYEL1O2je3XpP1eSfc19a1ySpi zSrUhlYD1ukR71IETZTrpScZAehaJjq35Pccd2mxqtxTGqyk7W1FtV0tO2n3jtV3yR2j zDSw== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@google.com header.s=20161025 header.b=m/FetFdA; dkim=neutral (body hash did not verify) header.i=@chromium.org header.s=google header.b=bGWfB2Co; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=chromium.org Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id z93si6094926wrc.226.2017.08.25.12.06.48; Fri, 25 Aug 2017 12:06:50 -0700 (PDT) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@google.com header.s=20161025 header.b=m/FetFdA; dkim=neutral (body hash did not verify) header.i=@chromium.org header.s=google header.b=bGWfB2Co; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 48201689C89; Fri, 25 Aug 2017 22:06:37 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm0-f47.google.com (mail-wm0-f47.google.com [74.125.82.47]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 17A6168075B for ; Fri, 25 Aug 2017 22:06:30 +0300 (EEST) Received: by mail-wm0-f47.google.com with SMTP id b189so5040341wmd.0 for ; Fri, 25 Aug 2017 12:06:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to; bh=rYG8pGV8TxuWYiuuG9Q94rAJVIcNOPwRqvtlGZ2YlIA=; b=m/FetFdAdMRT8Dy84OjZDIE/hN6DfNhO96bmR/hODTO9DDWMLK0nt3lijTYdFwjnbp vBj2DcLC9pTknei+6r1J5Z8cz8IL2n5l22qGaezgsNxoWYJYNH/Yvoo2mZ2Ygr2g5WbM tUFQylWTkrKAIjAErCLYCL0kcASlLYzmJ28L/ey6s3kdRwZ6RZv+jsAjptuECsCnLpNX lP78JLNV8B2j3UrNAFM2BaQ9sfBRnNkU6/QAEtj1d8BZ8nhOeQFdp+saXeXt4tDvgyRV 3bg0/Y48vgAUD9pfbdLeSRVPNq8clFTnoZIIS4rE5rHmS6O2dHo/kBKnRxH1VgEhx7Bs 85rA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to; bh=rYG8pGV8TxuWYiuuG9Q94rAJVIcNOPwRqvtlGZ2YlIA=; b=bGWfB2CoSWxxALn3L8sIvGVHunNKR/GNYVLRZu0GAd2N+nQGBcKUX1Odw4hxAdm1xc j7PD3O91kqLnQ3vPzxFmKygwnGWt+kAkF3te4PQzT8uRu/Q0tJPt4pFXaKDjM5RsOzn3 mUP/uVSPJigGWlgiOtP24Cy+RB7xQbPl/34CQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to; bh=rYG8pGV8TxuWYiuuG9Q94rAJVIcNOPwRqvtlGZ2YlIA=; b=r3y7CfzURgUyI2DjaLBaEwuXL5URBgkE7sOeN0b5DFgZOm8vdVw/nn52h++02IQk+5 ieiQ+2Jr37wUVQ4txrMDOVjWxaFGzeoXoJ29J2A6SKDRslhaT68k30oLzSsqY3VUgB0o eytJ698GLxVIw2Vjk0fmyfcMvn9R2K5GsBvBertYVG/6w9GLU3ffUqxsXViTuYUjVcUJ 0zSAPP6ZX+gxEdUpWIiRjqYCAJOGPTTgH4L9ddOHLuTAWNt2KrXa7RXBxkC2jrxYuGYX grmDHWH2ox19V4qTVEWn3bEC7ooVwLJB+kNRusfsdN1L4rW1y0VMQwtuck7XRkZIVc7z SysQ== X-Gm-Message-State: AHYfb5goJuitF5XDQaMBGr4+VZqRHdmzyPCHMQOOzZBxtZG6UUmgukwR J7dy9zd51YxaBFRq1TUQhH3RNSj//2WrkjU= X-Google-Smtp-Source: ADKCNb5kO6j6LXxUr5kVVsbvND9WyNi/nltXWk55s+mYX3gSy82MVsnRHfNVDaisEXKhIBp6KoiSm9iYBl5Ggn+kGEc= X-Received: by 10.28.191.157 with SMTP id o29mr182869wmi.121.1503687612399; Fri, 25 Aug 2017 12:00:12 -0700 (PDT) MIME-Version: 1.0 Received: by 10.28.11.131 with HTTP; Fri, 25 Aug 2017 11:59:51 -0700 (PDT) In-Reply-To: <20170825124326.GL7094@nb4> References: <20170825124326.GL7094@nb4> From: Dale Curtis Date: Fri, 25 Aug 2017 11:59:51 -0700 X-Google-Sender-Auth: CQlKTNEwI-xbqc8ZUsCq54j-ySU Message-ID: To: FFmpeg development discussions and patches X-Content-Filtered-By: Mailman/MimeDel 2.1.20 Subject: Re: [FFmpeg-devel] [mov] Bail when invalid sample data is present. 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 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" On Fri, Aug 25, 2017 at 5:43 AM, Michael Niedermayer wrote: > > This patch breaks: > http://samples.ffmpeg.org/mov/mp4/discont-frag.mp4 > > Hmm, indeed it does. The reason is that we read the sample count from the stsz box and then read the trun box. I don't think this block of code has ever been correct in that case: http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavformat/mov.c;hb=HEAD# l4287 It shifts all the ctts entries incorrectly and even did so prior to my patch. I've uploaded a new version of my fix which simply deletes this block of code. It passes all the fate test cases and those we have in Chrome. Let me know if fails any of your private test cases. - dale From 049f885ee972b0efb6dcbf456025e56dd627b8b9 Mon Sep 17 00:00:00 2001 From: Dale Curtis Date: Mon, 31 Jul 2017 13:44:22 -0700 Subject: [PATCH] [mov] Bail when invalid sample data is present. ctts data in ffmpeg relies on the index entries array to be 1:1 with samples... yet sc->sample_count can be read directly from the 'stsz' box and index entries are only generated if a chunk count has been read from 'stco' box. Ensure that if sc->sample_count > 0, sc->chunk_count is too as a basic sanity check. Additionally we need to check that after the index is built we have the right number of entries, so we also check in mov_read_trun() that sc->sample_count == st->nb_index_entries. --- libavformat/mov.c | 25 +++---------------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/libavformat/mov.c b/libavformat/mov.c index 876f48d912..f8bcba4cd1 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -3755,8 +3755,9 @@ static int mov_read_trak(MOVContext *c, AVIOContext *pb, MOVAtom atom) c->trak_index = -1; /* sanity checks */ - if (sc->chunk_count && (!sc->stts_count || !sc->stsc_count || - (!sc->sample_size && !sc->sample_count))) { + if ((sc->chunk_count && (!sc->stts_count || !sc->stsc_count || + (!sc->sample_size && !sc->sample_count))) || + (!sc->chunk_count && sc->sample_count)) { av_log(c->fc, AV_LOG_ERROR, "stream %d, missing mandatory atoms, broken header\n", st->index); return 0; @@ -4284,26 +4285,6 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom) entries = avio_rb32(pb); av_log(c->fc, AV_LOG_TRACE, "flags 0x%x entries %u\n", flags, entries); - /* Always assume the presence of composition time offsets. - * Without this assumption, for instance, we cannot deal with a track in fragmented movies that meet the following. - * 1) in the initial movie, there are no samples. - * 2) in the first movie fragment, there is only one sample without composition time offset. - * 3) in the subsequent movie fragments, there are samples with composition time offset. */ - if (!sc->ctts_count && sc->sample_count) - { - /* Complement ctts table if moov atom doesn't have ctts atom. */ - ctts_data = av_fast_realloc(NULL, &sc->ctts_allocated_size, sizeof(*sc->ctts_data) * sc->sample_count); - if (!ctts_data) - return AVERROR(ENOMEM); - /* Don't use a count greater than 1 here since it will leave a gap in - * the ctts index which the code below relies on being sequential. */ - sc->ctts_data = ctts_data; - for (i = 0; i < sc->sample_count; i++) { - sc->ctts_data[sc->ctts_count].count = 1; - sc->ctts_data[sc->ctts_count].duration = 0; - sc->ctts_count++; - } - } if ((uint64_t)entries+sc->ctts_count >= UINT_MAX/sizeof(*sc->ctts_data)) return AVERROR_INVALIDDATA; if (flags & MOV_TRUN_DATA_OFFSET) data_offset = avio_rb32(pb); -- 2.14.1.342.g6490525c54-goog