diff mbox

[FFmpeg-devel,mov] Bail when invalid sample data is present.

Message ID CAPUDrwcVKR0VEfX9pG=o0f8NUx9-OFamPsWAPdrMxajvQBs2jw@mail.gmail.com
State Superseded
Headers show

Commit Message

Dale Curtis July 31, 2017, 9:40 p.m. UTC
[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.

Comments

Carl Eugen Hoyos Aug. 2, 2017, 8:43 p.m. UTC | #1
2017-07-31 23:40 GMT+02:00 Dale Curtis <dalecurtis@chromium.org>:
> [mov] Bail when invalid sample data is present.

Could you provide such a sample?

Could the patch stop demuxing samples that are
supported so far?

Carl Eugen
Dale Curtis Aug. 4, 2017, 11:04 p.m. UTC | #2
Sample sent privately. I didn't find any non-fuzzer samples that no longer
demux in our Chrome test corpus or in fate.

- dale

On Wed, Aug 2, 2017 at 1:43 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:

> 2017-07-31 23:40 GMT+02:00 Dale Curtis <dalecurtis@chromium.org>:
> > [mov] Bail when invalid sample data is present.
>
> Could you provide such a sample?
>
> Could the patch stop demuxing samples that are
> supported so far?
>
> Carl Eugen
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Dale Curtis Aug. 25, 2017, 12:07 a.m. UTC | #3
+michael

This patch is also necessary now that you've applied the ctts fixes in the
previous patch. Thanks.

- dale

On Fri, Aug 4, 2017 at 4:04 PM, Dale Curtis <dalecurtis@chromium.org> wrote:

> Sample sent privately. I didn't find any non-fuzzer samples that no longer
> demux in our Chrome test corpus or in fate.
>
> - dale
>
> On Wed, Aug 2, 2017 at 1:43 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com>
> wrote:
>
>> 2017-07-31 23:40 GMT+02:00 Dale Curtis <dalecurtis@chromium.org>:
>> > [mov] Bail when invalid sample data is present.
>>
>> Could you provide such a sample?
>>
>> Could the patch stop demuxing samples that are
>> supported so far?
>>
>> Carl Eugen
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
>
diff mbox

Patch

From e3b51516046255540c5a76b41e02cee7f0902541 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.
---
 libavformat/mov.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index ab8e914581..5fe9bfac59 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -3750,8 +3750,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;
-- 
2.14.0.rc0.400.g1c36432dff-goog