[FFmpeg-devel] [PATCH] lavf/mov: ensure only one tkhd per trak

[Michael Niedermayer]

On Wed, Dec 12, 2018 at 08:08:53PM -0800, Baptiste Coudurier wrote:
> Hi,
> 
> > On Dec 12, 2018, at 6:53 PM, chcunningham <chcunningham at chromium.org> 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:
--- 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 */


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



[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20181213/d2ede5e8/attachment.sig>