From patchwork Thu Dec 13 12:05:36 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Michael Niedermayer X-Patchwork-Id: 11401 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 EEECA44C5DD for ; Thu, 13 Dec 2018 14:05:44 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 274CF68A8D4; Thu, 13 Dec 2018 14:05:45 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 4B0A768A8B8 for ; Thu, 13 Dec 2018 14:05:38 +0200 (EET) X-Originating-IP: 213.47.41.20 Received: from localhost (213-47-41-20.cable.dynamic.surfer.at [213.47.41.20]) (Authenticated sender: michael@niedermayer.cc) by relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 2CED11C0002 for ; Thu, 13 Dec 2018 12:05:36 +0000 (UTC) Date: Thu, 13 Dec 2018 13:05:36 +0100 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20181213120536.GL3501@michaelspb> References: <20181213025301.191821-1-chcunningham@chromium.org> <53461D42-D96C-442B-AB0E-8C9AFAC49CC3@gmail.com> MIME-Version: 1.0 In-Reply-To: <53461D42-D96C-442B-AB0E-8C9AFAC49CC3@gmail.com> User-Agent: Mutt/1.5.24 (2015-08-30) Subject: Re: [FFmpeg-devel] [PATCH] lavf/mov: ensure only one tkhd per trak 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 Wed, Dec 12, 2018 at 08:08:53PM -0800, Baptiste Coudurier wrote: > Hi, > > > On Dec 12, 2018, at 6:53 PM, chcunningham wrote: > > > > Chromium fuzzing produced a whacky file with extra tkhds. This caused > > an AVStream that was already in use to be corrupted by assigning it a > > new id, which blows up later in mov_read_trun because the > > MOVFragmentStreamInfo.index_entry now points OOB. > > --- > > libavformat/isom.h | 3 ++- > > libavformat/mov.c | 7 +++++++ > > 2 files changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/libavformat/isom.h b/libavformat/isom.h > > index e629663949..e14d670f2f 100644 > > --- a/libavformat/isom.h > > +++ b/libavformat/isom.h > > @@ -230,7 +230,8 @@ typedef struct MOVStreamContext { > > > > uint32_t format; > > > > - int has_sidx; // If there is an sidx entry for this stream. > > + int has_sidx; ///< If there is a sidx entry for this stream. > > + int has_tkhd; ///< If there is a tkhd entry for this stream. > > struct { > > struct AVAESCTR* aes_ctr; > > unsigned int per_sample_iv_size; // Either 0, 8, or 16. > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > index ec57a05803..47c53d7992 100644 > > --- a/libavformat/mov.c > > +++ b/libavformat/mov.c > > @@ -4438,6 +4438,12 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext *pb, MOVAtom atom) > > st = c->fc->streams[c->fc->nb_streams-1]; > > sc = st->priv_data; > > > > + // Each stream (trak) should have exactly 1 tkhd. This catches bad files and > > + // avoids corrupting AVStreams mapped to an earlier tkhd. > > + if (sc->has_tkhd) > > + return AVERROR_INVALIDDATA; > > + sc->has_tkhd = 1; > > + > > > > Could we just check that st->id is already set ? IIRC 0 is not a valid value I have at least 2 files which have a id of 0 Iam not sure where they are from so iam not sure i can share them found with: Input #0, mov,mp4,m4a,3gp,3g2,mj2, from '/home/michael/videos/qtrle/Earth Spin 1-bit qtrle.mov': Metadata: creation_time : 2015-12-20T17:51:06.000000Z copyright : ©Aroborescence, San Francisco All Rights Reserved. copyright-eng : ©Aroborescence, San Francisco All Rights Reserved. comment : Not for reuse or resale comment-eng : Not for reuse or resale date : Monday, May 6, 1991 date-eng : Monday, May 6, 1991 original_format : Digitized original_format-eng: Digitized director : director-eng : producer : producer-eng : composer : composer-eng : performers : performers-eng : original_source : original_source-eng: Duration: 00:00:03.60, start: 0.000000, bitrate: 203 kb/s Stream #0:0(eng): Video: qtrle (rle / 0x20656C72), pal8, 186x186, 197 kb/s, 10 fps, 10 tbr, 600 tbn, 600 tbc (default) Metadata: rotate : 0 creation_time : 2015-12-20T17:51:06.000000Z handler_name : Apple Alias Data Handler encoder : Animation - RLE Side data: displaymatrix: rotation of -0.00 degrees [...] --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -4450,6 +4450,7 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext *pb, MOVAtom atom) avio_rb32(pb); /* modification time */ } st->id = (int)avio_rb32(pb); /* track id (NOT 0 !)*/ + av_assert0(st->id); avio_rb32(pb); /* reserved */ /* highlevel (considering edits) duration in movie timebase */