| Message ID | 20190201011826.23766-1-chcunningham@chromium.org |
|---|---|
| State | Superseded |
| Headers | show |
Some extra context: this remedies some bad math that otherwise triggers an abort near the bottom of mov_seek_stream(). My first thought on seeing this was we should probably be returning AVERROR_INVALIDDATA somewhere. The stsc and stco boxes aren't agreeing (stco says no chunks, stsc.first points to chunk 135). stsc and stco aren't gauranteed to come in a particular order, but we could try to validate that they agree whenever both arrive. One challenge is that mov_read_stco is already pretty relaxed about setting sc->chunk_count (it sets to some lower number if we hit EOF during parsing). Feedback welcome. On Thu, Jan 31, 2019 at 5:26 PM chcunningham <chcunningham@chromium.org> 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. > > Also updates MOVStsc to use unsigned ints, per spec. > --- > libavformat/isom.h | 6 +++--- > libavformat/mov.c | 4 ++-- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/libavformat/isom.h b/libavformat/isom.h > index e629663949..8e0d8355b3 100644 > --- a/libavformat/isom.h > +++ b/libavformat/isom.h > @@ -59,9 +59,9 @@ typedef struct MOVStts { > } MOVStts; > > typedef struct MOVStsc { > - int first; > - int count; > - int id; > + unsigned int first; > + unsigned int count; > + unsigned int id; > } MOVStsc; > > typedef struct MOVElst { > 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; > -- > 2.20.1.611.gfbb209baf1-goog > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
2019-02-01 2:18 GMT+01:00, chcunningham <chcunningham@chromium.org>: > 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. > Also updates MOVStsc to use unsigned ints, per spec. Could be split... Carl Eugen
diff --git a/libavformat/isom.h b/libavformat/isom.h index e629663949..8e0d8355b3 100644 --- a/libavformat/isom.h +++ b/libavformat/isom.h @@ -59,9 +59,9 @@ typedef struct MOVStts { } MOVStts; typedef struct MOVStsc { - int first; - int count; - int id; + unsigned int first; + unsigned int count; + unsigned int id; } MOVStsc; typedef struct MOVElst { 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;