Re: [FFmpeg-devel] [PATCH] lavf/mov: fix huge alloc in mov_read_ctts

[Michael Niedermayer]

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

[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.

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

[Derek Buitenhuis]

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

[John Stebbins]

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

[James Almer]

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

[John Stebbins]

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

[Carl Eugen Hoyos]

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

[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.
---
 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