| Message ID | CAPUDrwfcFOfOyrD3XQuW929iyLHSe8A=EO_5r=jaAPH7XSrkTw@mail.gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, Jul 31, 2017 at 04:42:20PM -0700, Dale Curtis wrote: > I'm not convinced my original patch catches all cases. So here's an updated > one which explicitly verifies the contract. > > - dale > > On Mon, Jul 31, 2017 at 2:40 PM, Dale Curtis <dalecurtis@chromium.org> > wrote: > > > [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. > > > > This should be applied on top of the ctts fixes in my previous patch. > > > > > mov.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > b9e9d387abfa321d17f117833f0b4a6f04ab6feb sample_count_fix_v2.patch > From 51571dd294350f2ef367fd9391ed4c1e94387947 Mon Sep 17 00:00:00 2001 > From: Dale Curtis <dalecurtis@chromium.org> > 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 | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) This patch breaks: http://samples.ffmpeg.org/mov/mp4/discont-frag.mp4 [...]
From 51571dd294350f2ef367fd9391ed4c1e94387947 Mon Sep 17 00:00:00 2001 From: Dale Curtis <dalecurtis@chromium.org> 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 | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libavformat/mov.c b/libavformat/mov.c index 13b5e454d8..6edb898b3e 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -3752,8 +3752,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; @@ -4288,6 +4289,9 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom) * 3) in the subsequent movie fragments, there are samples with composition time offset. */ if (!sc->ctts_count && sc->sample_count) { + /* ctts relies on being 1:1 with sample entries. */ + if (sc->sample_count != st->nb_index_entries) + return AVERROR_INVALIDDATA; /* 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) -- 2.14.0.rc0.400.g1c36432dff-goog