On Wed, Dec 12, 2018 at 08:08:53PM -0800, Baptiste Coudurier wrote:
> Hi,
>
> > On Dec 12, 2018, at 6:53 PM, chcunningham <chcunning...@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
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
