| Message ID | 20190202022519.24932-1-chcunningham@chromium.org |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, Feb 01, 2019 at 06:25:19PM -0800, chcunningham wrote: > Bad content may contain stsc boxes with a first_chunk index that > exceeds stco.entries (chunk_count). > > mov_get_stsc_samples now checks for this and returns 0 when > values are invalid. > --- > libavformat/mov.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index 9b9739f788..dcf4ee8dc1 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -2690,11 +2690,11 @@ static inline int mov_stsc_index_valid(unsigned int index, unsigned int count) > /* Compute the samples value for the stsc entry at the given index. */ > static inline int64_t mov_get_stsc_samples(MOVStreamContext *sc, unsigned int index) > { > - int chunk_count; > + unsigned int chunk_count = 0; > > if (mov_stsc_index_valid(index, sc->stsc_count)) > chunk_count = sc->stsc_data[index + 1].first - sc->stsc_data[index].first; > - else > + else if (sc->chunk_count >= sc->stsc_data[index].first) > chunk_count = sc->chunk_count - (sc->stsc_data[index].first - 1); This construct occurs a 2nd time (in mov_build_index()) is this not affected? mov_read_trak() contains a check for chunk_count and the first index(es) (obviously this is not catching this one but) is there a reason not to eliminate the inconsistancy at that or some other "early" point? Thanks [...]
On Sat, Feb 2, 2019 at 3:55 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > > static inline int64_t mov_get_stsc_samples(MOVStreamContext *sc, > unsigned int index) > > { > > - int chunk_count; > > + unsigned int chunk_count = 0; > > > > if (mov_stsc_index_valid(index, sc->stsc_count)) > > chunk_count = sc->stsc_data[index + 1].first - > sc->stsc_data[index].first; > > - else > > + else if (sc->chunk_count >= sc->stsc_data[index].first) > > chunk_count = sc->chunk_count - (sc->stsc_data[index].first - > 1); > > This construct occurs a 2nd time (in mov_build_index()) is this not > affected? > Didn't notice it, but I think it would be affected. I'll leave this alone for now, but I'm open to adding a mov_get_chunk_count helper to call from both mov_build_index and mov_get_stsc_samples. > mov_read_trak() contains a check for chunk_count and the first index(es) > (obviously this is not catching this one but) > is there a reason not to eliminate the inconsistancy at that or some other > "early" point? > Agree this sounds better. Stand by for patch.
diff --git a/libavformat/mov.c b/libavformat/mov.c index 9b9739f788..dcf4ee8dc1 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -2690,11 +2690,11 @@ static inline int mov_stsc_index_valid(unsigned int index, unsigned int count) /* Compute the samples value for the stsc entry at the given index. */ static inline int64_t mov_get_stsc_samples(MOVStreamContext *sc, unsigned int index) { - int chunk_count; + unsigned int chunk_count = 0; if (mov_stsc_index_valid(index, sc->stsc_count)) chunk_count = sc->stsc_data[index + 1].first - sc->stsc_data[index].first; - else + else if (sc->chunk_count >= sc->stsc_data[index].first) chunk_count = sc->chunk_count - (sc->stsc_data[index].first - 1); return sc->stsc_data[index].count * (int64_t)chunk_count;