Re: [FFmpeg-devel] [PATCH] lavf/mov: fix huge alloc in mov_read_ctts
On Sun, Nov 26, 2017 at 07:32:30AM -0800, John Stebbins wrote: > An invalid file may cause huge alloc. Delay expansion of ctts entries > until the number of samples is known in mov_build_index. > > Found-by: zhao dongzhuo, AD-lab of Venustech > --- > libavformat/mov.c | 31 +++ > 1 file changed, 27 insertions(+), 4 deletions(-) will apply thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The worst form of inequality is to try to make unequal things equal. -- Aristotle signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavf/mov: fix huge alloc in mov_read_ctts
An invalid file may cause huge alloc. Delay expansion of ctts entries until the number of samples is known in mov_build_index. Found-by: zhao dongzhuo, AD-lab of Venustech --- libavformat/mov.c | 31 +++ 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/libavformat/mov.c b/libavformat/mov.c index ddb1e59b85..7a7fd13099 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -2896,7 +2896,7 @@ static int mov_read_ctts(MOVContext *c, AVIOContext *pb, MOVAtom atom) { AVStream *st; MOVStreamContext *sc; -unsigned int i, j, entries, ctts_count = 0; +unsigned int i, entries, ctts_count = 0; if (c->fc->nb_streams < 1) return 0; @@ -2929,9 +2929,8 @@ static int mov_read_ctts(MOVContext *c, AVIOContext *pb, MOVAtom atom) continue; } -/* Expand entries such that we have a 1-1 mapping with samples. */ -for (j = 0; j < count; j++) -add_ctts_entry(>ctts_data, _count, >ctts_allocated_size, 1, duration); +add_ctts_entry(>ctts_data, _count, >ctts_allocated_size, + count, duration); av_log(c->fc, AV_LOG_TRACE, "count=%d, duration=%d\n", count, duration); @@ -3580,6 +3579,8 @@ static void mov_build_index(MOVContext *mov, AVStream *st) unsigned int stps_index = 0; unsigned int i, j; uint64_t stream_size = 0; +MOVStts *ctts_data_old = sc->ctts_data; +unsigned int ctts_count_old = sc->ctts_count; if (sc->elst_count) { int i, edit_start_index = 0, multiple_edits = 0; @@ -3648,6 +3649,28 @@ static void mov_build_index(MOVContext *mov, AVStream *st) } st->index_entries_allocated_size = (st->nb_index_entries + sc->sample_count) * sizeof(*st->index_entries); +if (ctts_data_old) { +// Expand ctts entries such that we have a 1-1 mapping with samples +if (sc->sample_count >= UINT_MAX / sizeof(*sc->ctts_data)) +return; +sc->ctts_count = 0; +sc->ctts_allocated_size = 0; +sc->ctts_data = av_fast_realloc(NULL, >ctts_allocated_size, +sc->sample_count * sizeof(*sc->ctts_data)); +if (!sc->ctts_data) { +av_free(ctts_data_old); +return; +} +for (i = 0; i < ctts_count_old && +sc->ctts_count < sc->sample_count; i++) +for (j = 0; j < ctts_data_old[i].count && +sc->ctts_count < sc->sample_count; j++) +add_ctts_entry(>ctts_data, >ctts_count, + >ctts_allocated_size, 1, + ctts_data_old[i].duration); +av_free(ctts_data_old); +} + for (i = 0; i < sc->chunk_count; i++) { int64_t next_offset = i+1 < sc->chunk_count ? sc->chunk_offsets[i+1] : INT64_MAX; current_offset = sc->chunk_offsets[i]; -- 2.14.3 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/mov: fix huge alloc in mov_read_ctts
On 11/26/2017 3:10 PM, John Stebbins wrote: > Is there some git magic for this, or is this just something you add manually > to the bottom of the commit message? It's just manual. - Derek ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/mov: fix huge alloc in mov_read_ctts
On 11/25/2017 05:03 PM, James Almer wrote: > On 11/25/2017 10:00 PM, John Stebbins wrote: >> On 11/25/2017 04:03 PM, Carl Eugen Hoyos wrote: >>> 2017-11-25 21:11 GMT+01:00 John Stebbins: An invalid file may cause huge alloc. Delay expansion of ctts entries until the number of samples is known in mov_build_index. >>> Please mention zhao dongzhuo from ADlab of Venustech who found this >>> issue, I can confirm that the memory allocation gets fixed. >>> >>> >> Sure. Should I amend the commit message to add something like, "Thanks to >> zhao dongzhuo from ADlab of Venustech for >> reporting this issue"? > Or just a "Found-by:" line. > > Is there some git magic for this, or is this just something you add manually to the bottom of the commit message? -- John GnuPG fingerprint: D0EC B3DB C372 D1F1 0B01 83F0 49F1 D7B2 60D4 D0F7 signature.asc Description: OpenPGP digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/mov: fix huge alloc in mov_read_ctts
On 11/25/2017 10:00 PM, John Stebbins wrote: > On 11/25/2017 04:03 PM, Carl Eugen Hoyos wrote: >> 2017-11-25 21:11 GMT+01:00 John Stebbins: >>> An invalid file may cause huge alloc. Delay expansion of ctts entries >>> until the number of samples is known in mov_build_index. >> Please mention zhao dongzhuo from ADlab of Venustech who found this >> issue, I can confirm that the memory allocation gets fixed. >> >> > > Sure. Should I amend the commit message to add something like, "Thanks to > zhao dongzhuo from ADlab of Venustech for > reporting this issue"? Or just a "Found-by:" line. See commit 837cb4325b712ff1aab531bf41668933f61d75d2 for an example. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/mov: fix huge alloc in mov_read_ctts
On 11/25/2017 04:03 PM, Carl Eugen Hoyos wrote: > 2017-11-25 21:11 GMT+01:00 John Stebbins: >> An invalid file may cause huge alloc. Delay expansion of ctts entries >> until the number of samples is known in mov_build_index. > Please mention zhao dongzhuo from ADlab of Venustech who found this > issue, I can confirm that the memory allocation gets fixed. > > Sure. Should I amend the commit message to add something like, "Thanks to zhao dongzhuo from ADlab of Venustech for reporting this issue"? -- John GnuPG fingerprint: D0EC B3DB C372 D1F1 0B01 83F0 49F1 D7B2 60D4 D0F7 signature.asc Description: OpenPGP digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/mov: fix huge alloc in mov_read_ctts
2017-11-25 21:11 GMT+01:00 John Stebbins: > An invalid file may cause huge alloc. Delay expansion of ctts entries > until the number of samples is known in mov_build_index. Please mention zhao dongzhuo from ADlab of Venustech who found this issue, I can confirm that the memory allocation gets fixed. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavf/mov: fix huge alloc in mov_read_ctts
An invalid file may cause huge alloc. Delay expansion of ctts entries until the number of samples is known in mov_build_index. --- libavformat/mov.c | 31 +++ 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/libavformat/mov.c b/libavformat/mov.c index ddb1e59b85..7a7fd13099 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -2896,7 +2896,7 @@ static int mov_read_ctts(MOVContext *c, AVIOContext *pb, MOVAtom atom) { AVStream *st; MOVStreamContext *sc; -unsigned int i, j, entries, ctts_count = 0; +unsigned int i, entries, ctts_count = 0; if (c->fc->nb_streams < 1) return 0; @@ -2929,9 +2929,8 @@ static int mov_read_ctts(MOVContext *c, AVIOContext *pb, MOVAtom atom) continue; } -/* Expand entries such that we have a 1-1 mapping with samples. */ -for (j = 0; j < count; j++) -add_ctts_entry(>ctts_data, _count, >ctts_allocated_size, 1, duration); +add_ctts_entry(>ctts_data, _count, >ctts_allocated_size, + count, duration); av_log(c->fc, AV_LOG_TRACE, "count=%d, duration=%d\n", count, duration); @@ -3580,6 +3579,8 @@ static void mov_build_index(MOVContext *mov, AVStream *st) unsigned int stps_index = 0; unsigned int i, j; uint64_t stream_size = 0; +MOVStts *ctts_data_old = sc->ctts_data; +unsigned int ctts_count_old = sc->ctts_count; if (sc->elst_count) { int i, edit_start_index = 0, multiple_edits = 0; @@ -3648,6 +3649,28 @@ static void mov_build_index(MOVContext *mov, AVStream *st) } st->index_entries_allocated_size = (st->nb_index_entries + sc->sample_count) * sizeof(*st->index_entries); +if (ctts_data_old) { +// Expand ctts entries such that we have a 1-1 mapping with samples +if (sc->sample_count >= UINT_MAX / sizeof(*sc->ctts_data)) +return; +sc->ctts_count = 0; +sc->ctts_allocated_size = 0; +sc->ctts_data = av_fast_realloc(NULL, >ctts_allocated_size, +sc->sample_count * sizeof(*sc->ctts_data)); +if (!sc->ctts_data) { +av_free(ctts_data_old); +return; +} +for (i = 0; i < ctts_count_old && +sc->ctts_count < sc->sample_count; i++) +for (j = 0; j < ctts_data_old[i].count && +sc->ctts_count < sc->sample_count; j++) +add_ctts_entry(>ctts_data, >ctts_count, + >ctts_allocated_size, 1, + ctts_data_old[i].duration); +av_free(ctts_data_old); +} + for (i = 0; i < sc->chunk_count; i++) { int64_t next_offset = i+1 < sc->chunk_count ? sc->chunk_offsets[i+1] : INT64_MAX; current_offset = sc->chunk_offsets[i]; -- 2.14.3 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel