On 12/13/18, Michael Niedermayer <mich...@niedermayer.cc> wrote:
> 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
Haha.
> 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
>
Indeed.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
