[FFmpeg-devel] [PATCH 1/4] avformat/mov: Avoid cloning encryption info if its unchanged

[FFmpeg-devel] [PATCH 1/4] avformat/mov: Avoid cloning encryption info if its unchanged

[Michael Niedermayer]

Fixes: OOM
Fixes: 45834/clusterfuzz-testcase-minimized-ffmpeg_dem_MOV_fuzzer-5419540462305280

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavformat/mov.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 3ec0ea2361..c93e13c8cd 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -6187,9 +6187,12 @@ static int mov_read_sample_encryption_info(MOVContext *c, AVIOContext *pb, MOVSt
         return AVERROR_INVALIDDATA;
     }
 
-    *sample = av_encryption_info_clone(sc->cenc.default_encrypted_sample);
-    if (!*sample)
-        return AVERROR(ENOMEM);
+    if (sc->cenc.per_sample_iv_size || use_subsamples) {
+        *sample = av_encryption_info_clone(sc->cenc.default_encrypted_sample);
+        if (!*sample)
+            return AVERROR(ENOMEM);
+    } else
+        *sample = NULL;
 
     if (sc->cenc.per_sample_iv_size != 0) {
         if ((ret = ffio_read_size(pb, (*sample)->iv, sc->cenc.per_sample_iv_size)) < 0) {
@@ -7120,6 +7123,8 @@ static int cenc_filter(MOVContext *mov, AVStream* st, MOVStreamContext *sc, AVPa
         } else if (encrypted_index >= 0 && encrypted_index < encryption_index->nb_encrypted_samples) {
             // Per-sample setting override.
             encrypted_sample = encryption_index->encrypted_samples[encrypted_index];
+            if (!encrypted_sample)
+                encrypted_sample = sc->cenc.default_encrypted_sample;
         } else {
             av_log(mov->fc, AV_LOG_ERROR, "Incorrect number of samples in encryption info\n");
             return AVERROR_INVALIDDATA;
-- 
2.17.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/4] avformat/mov: Add special case for slow duplication loop in mov_read_trun()

[Michael Niedermayer]

This extra code is ugly, better solution is welcome

Fixes: Timeout
Fixes: 45700/clusterfuzz-testcase-minimized-ffmpeg_IO_DEMUXER_fuzzer-6141847792123904

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavformat/mov.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index c93e13c8cd..3d9e866d4e 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -5092,6 +5092,7 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     if (index_entry_pos > 0)
         prev_dts = sti->index_entries[index_entry_pos-1].timestamp;
 
+    if (flags & 0xF00) {
     for (i = 0; i < entries && !pb->eof_reached; i++) {
         unsigned sample_size = frag->size;
         int sample_flags = i ? frag->flags : first_sample_flags;
@@ -5166,6 +5167,74 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom)
             sc->nb_frames_for_fps ++;
         }
     }
+    } else {
+        unsigned sample_size = frag->size;
+        unsigned sample_duration = frag->duration;
+
+        if (pts != AV_NOPTS_VALUE) {
+            dts = pts - sc->dts_shift - sc->time_offset;
+            av_log(c->fc, AV_LOG_DEBUG,
+                "pts %"PRId64" calculated dts %"PRId64
+                " sc->dts_shift %d ctts.duration %d"
+                " sc->time_offset %"PRId64
+                " flags & MOV_TRUN_SAMPLE_CTS %d\n",
+                pts, dts,
+                sc->dts_shift, 0,
+                sc->time_offset, 0);
+        }
+
+        if (av_sat_add64(dts, sample_duration * entries) != dts + (uint64_t)sample_duration * entries)
+            return AVERROR_INVALIDDATA;
+
+        for (i = 0; i < entries && !pb->eof_reached; i++) {
+            int sample_flags = i ? frag->flags : first_sample_flags;
+            int keyframe = 0;
+            int index_entry_flags = 0;
+
+            if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO)
+                keyframe = 1;
+            else
+                keyframe =
+                    !(sample_flags & (MOV_FRAG_SAMPLE_FLAG_IS_NON_SYNC |
+                                    MOV_FRAG_SAMPLE_FLAG_DEPENDS_YES));
+            if (keyframe) {
+                distance = 0;
+                index_entry_flags |= AVINDEX_KEYFRAME;
+            }
+            // Fragments can overlap in time.  Discard overlapping frames after
+            // decoding.
+            if (prev_dts >= dts)
+                index_entry_flags |= AVINDEX_DISCARD_FRAME;
+
+            sti->index_entries[index_entry_pos].pos   = offset;
+            sti->index_entries[index_entry_pos].timestamp = dts;
+            sti->index_entries[index_entry_pos].size  = sample_size;
+            sti->index_entries[index_entry_pos].min_distance = distance;
+            sti->index_entries[index_entry_pos].flags = index_entry_flags;
+
+            sc->ctts_data[index_entry_pos].count = 1;
+            sc->ctts_data[index_entry_pos].duration = 0;
+            index_entry_pos++;
+
+            av_log(c->fc, AV_LOG_TRACE, "AVIndex stream %d, sample %d, offset %"PRIx64", dts %"PRId64", "
+                    "size %u, distance %d, keyframe %d\n", st->index,
+                    index_entry_pos, offset, dts, sample_size, distance, keyframe);
+            distance++;
+            dts += sample_duration;
+            offset += sample_size;
+            sc->data_size += sample_size;
+
+            if (sample_duration <= INT64_MAX - sc->duration_for_fps &&
+                1 <= INT_MAX - sc->nb_frames_for_fps
+            ) {
+                sc->duration_for_fps += sample_duration;
+                sc->nb_frames_for_fps ++;
+            }
+        }
+
+    }
+
+
     if (frag_stream_info)
         frag_stream_info->next_trun_dts = dts + sc->time_offset;
     if (i < entries) {
-- 
2.17.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/4] avformat/mov: Add special case for slow duplication loop in mov_read_trun()

[Marton Balint]

On Sat, 18 Jun 2022, Michael Niedermayer wrote:

> This extra code is ugly, better solution is welcome

If you work on fixing these issues, it is kind of your job to find an 
elegant and maintainable solution. If you can't find one with reasonable 
amount of work, then IMHO it is better to leave the timeout issue in the 
code.

Regards,
Marton

>
> Fixes: Timeout
> Fixes: 45700/clusterfuzz-testcase-minimized-ffmpeg_IO_DEMUXER_fuzzer-6141847792123904
>
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
> libavformat/mov.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 69 insertions(+)
>
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index c93e13c8cd..3d9e866d4e 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -5092,6 +5092,7 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>     if (index_entry_pos > 0)
>         prev_dts = sti->index_entries[index_entry_pos-1].timestamp;
>
> +    if (flags & 0xF00) {
>     for (i = 0; i < entries && !pb->eof_reached; i++) {
>         unsigned sample_size = frag->size;
>         int sample_flags = i ? frag->flags : first_sample_flags;
> @@ -5166,6 +5167,74 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>             sc->nb_frames_for_fps ++;
>         }
>     }
> +    } else {
> +        unsigned sample_size = frag->size;
> +        unsigned sample_duration = frag->duration;
> +
> +        if (pts != AV_NOPTS_VALUE) {
> +            dts = pts - sc->dts_shift - sc->time_offset;
> +            av_log(c->fc, AV_LOG_DEBUG,
> +                "pts %"PRId64" calculated dts %"PRId64
> +                " sc->dts_shift %d ctts.duration %d"
> +                " sc->time_offset %"PRId64
> +                " flags & MOV_TRUN_SAMPLE_CTS %d\n",
> +                pts, dts,
> +                sc->dts_shift, 0,
> +                sc->time_offset, 0);
> +        }
> +
> +        if (av_sat_add64(dts, sample_duration * entries) != dts + (uint64_t)sample_duration * entries)
> +            return AVERROR_INVALIDDATA;
> +
> +        for (i = 0; i < entries && !pb->eof_reached; i++) {
> +            int sample_flags = i ? frag->flags : first_sample_flags;
> +            int keyframe = 0;
> +            int index_entry_flags = 0;
> +
> +            if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO)
> +                keyframe = 1;
> +            else
> +                keyframe =
> +                    !(sample_flags & (MOV_FRAG_SAMPLE_FLAG_IS_NON_SYNC |
> +                                    MOV_FRAG_SAMPLE_FLAG_DEPENDS_YES));
> +            if (keyframe) {
> +                distance = 0;
> +                index_entry_flags |= AVINDEX_KEYFRAME;
> +            }
> +            // Fragments can overlap in time.  Discard overlapping frames after
> +            // decoding.
> +            if (prev_dts >= dts)
> +                index_entry_flags |= AVINDEX_DISCARD_FRAME;
> +
> +            sti->index_entries[index_entry_pos].pos   = offset;
> +            sti->index_entries[index_entry_pos].timestamp = dts;
> +            sti->index_entries[index_entry_pos].size  = sample_size;
> +            sti->index_entries[index_entry_pos].min_distance = distance;
> +            sti->index_entries[index_entry_pos].flags = index_entry_flags;
> +
> +            sc->ctts_data[index_entry_pos].count = 1;
> +            sc->ctts_data[index_entry_pos].duration = 0;
> +            index_entry_pos++;
> +
> +            av_log(c->fc, AV_LOG_TRACE, "AVIndex stream %d, sample %d, offset %"PRIx64", dts %"PRId64", "
> +                    "size %u, distance %d, keyframe %d\n", st->index,
> +                    index_entry_pos, offset, dts, sample_size, distance, keyframe);
> +            distance++;
> +            dts += sample_duration;
> +            offset += sample_size;
> +            sc->data_size += sample_size;
> +
> +            if (sample_duration <= INT64_MAX - sc->duration_for_fps &&
> +                1 <= INT_MAX - sc->nb_frames_for_fps
> +            ) {
> +                sc->duration_for_fps += sample_duration;
> +                sc->nb_frames_for_fps ++;
> +            }
> +        }
> +
> +    }
> +
> +
>     if (frag_stream_info)
>         frag_stream_info->next_trun_dts = dts + sc->time_offset;
>     if (i < entries) {
> -- 
> 2.17.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/4] avformat/mov: Add special case for slow duplication loop in mov_read_trun()

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 738 bytes --]

On Sun, Jun 19, 2022 at 12:17:54AM +0200, Marton Balint wrote:
> 
> 
> On Sat, 18 Jun 2022, Michael Niedermayer wrote:
> 
> > This extra code is ugly, better solution is welcome
> 
> If you work on fixing these issues, it is kind of your job to find an
> elegant and maintainable solution. If you can't find one with reasonable
> amount of work, then IMHO it is better to leave the timeout issue in the
> code.

yes, you are correct
still such patches should be posted to the mailing list

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

"Nothing to hide" only works if the folks in power share the values of
you and everyone you know entirely and always will -- Tom Scott


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 3/4] avcodec/aasc: Fix indention

[Michael Niedermayer]

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/aasc.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/libavcodec/aasc.c b/libavcodec/aasc.c
index d297cfad76..528ee58cdf 100644
--- a/libavcodec/aasc.c
+++ b/libavcodec/aasc.c
@@ -104,26 +104,26 @@ static int aasc_decode_frame(AVCodecContext *avctx, AVFrame *rframe,
         ff_msrle_decode(avctx, s->frame, 8, &s->gb);
         break;
     case MKTAG('A', 'A', 'S', 'C'):
-    switch (compr) {
-    case 0:
-        stride = (avctx->width * psize + psize) & ~psize;
-        if (buf_size < stride * avctx->height)
+        switch (compr) {
+        case 0:
+            stride = (avctx->width * psize + psize) & ~psize;
+            if (buf_size < stride * avctx->height)
+                return AVERROR_INVALIDDATA;
+            for (i = avctx->height - 1; i >= 0; i--) {
+                memcpy(s->frame->data[0] + i * s->frame->linesize[0], buf, avctx->width * psize);
+                buf += stride;
+                buf_size -= stride;
+            }
+            break;
+        case 1:
+            bytestream2_init(&s->gb, buf, buf_size);
+            ff_msrle_decode(avctx, s->frame, 8, &s->gb);
+            break;
+        default:
+            av_log(avctx, AV_LOG_ERROR, "Unknown compression type %d\n", compr);
             return AVERROR_INVALIDDATA;
-        for (i = avctx->height - 1; i >= 0; i--) {
-            memcpy(s->frame->data[0] + i * s->frame->linesize[0], buf, avctx->width * psize);
-            buf += stride;
-            buf_size -= stride;
         }
         break;
-    case 1:
-        bytestream2_init(&s->gb, buf, buf_size);
-        ff_msrle_decode(avctx, s->frame, 8, &s->gb);
-        break;
-    default:
-        av_log(avctx, AV_LOG_ERROR, "Unknown compression type %d\n", compr);
-        return AVERROR_INVALIDDATA;
-    }
-        break;
     default:
         av_log(avctx, AV_LOG_ERROR, "Unknown FourCC: %X\n", avctx->codec_tag);
         return -1;
-- 
2.17.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 4/4] tools/target_dec_fuzzer: Adjust threshold for AASC

[Michael Niedermayer]

Fixes: Timeout
Fixes: 47919/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AASC_fuzzer-5176435830030336

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 tools/target_dec_fuzzer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/target_dec_fuzzer.c b/tools/target_dec_fuzzer.c
index fefc8514f0..3450e60eea 100644
--- a/tools/target_dec_fuzzer.c
+++ b/tools/target_dec_fuzzer.c
@@ -208,6 +208,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
     maxpixels = maxpixels_per_frame * maxiteration;
     maxsamples = maxsamples_per_frame * maxiteration;
     switch (c->p.id) {
+    case AV_CODEC_ID_AASC:        maxpixels  /= 1024;  break;
     case AV_CODEC_ID_AGM:         maxpixels  /= 1024;  break;
     case AV_CODEC_ID_ARBC:        maxpixels  /= 1024;  break;
     case AV_CODEC_ID_BINKVIDEO:   maxpixels  /= 32;    break;
-- 
2.17.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/4] avformat/mov: Avoid cloning encryption info if its unchanged

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 730 bytes --]

On Sat, Jun 18, 2022 at 10:03:00PM +0200, Michael Niedermayer wrote:
> Fixes: OOM
> Fixes: 45834/clusterfuzz-testcase-minimized-ffmpeg_dem_MOV_fuzzer-5419540462305280
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavformat/mov.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)

sorry for the mistaken resend, please ignore this patchset

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 1/4] avformat/mov: Avoid cloning encryption info if its unchanged

[Michael Niedermayer]

Fixes: OOM
Fixes: 45834/clusterfuzz-testcase-minimized-ffmpeg_dem_MOV_fuzzer-5419540462305280

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavformat/mov.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 3ec0ea2361..c93e13c8cd 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -6187,9 +6187,12 @@ static int mov_read_sample_encryption_info(MOVContext *c, AVIOContext *pb, MOVSt
         return AVERROR_INVALIDDATA;
     }
 
-    *sample = av_encryption_info_clone(sc->cenc.default_encrypted_sample);
-    if (!*sample)
-        return AVERROR(ENOMEM);
+    if (sc->cenc.per_sample_iv_size || use_subsamples) {
+        *sample = av_encryption_info_clone(sc->cenc.default_encrypted_sample);
+        if (!*sample)
+            return AVERROR(ENOMEM);
+    } else
+        *sample = NULL;
 
     if (sc->cenc.per_sample_iv_size != 0) {
         if ((ret = ffio_read_size(pb, (*sample)->iv, sc->cenc.per_sample_iv_size)) < 0) {
@@ -7120,6 +7123,8 @@ static int cenc_filter(MOVContext *mov, AVStream* st, MOVStreamContext *sc, AVPa
         } else if (encrypted_index >= 0 && encrypted_index < encryption_index->nb_encrypted_samples) {
             // Per-sample setting override.
             encrypted_sample = encryption_index->encrypted_samples[encrypted_index];
+            if (!encrypted_sample)
+                encrypted_sample = sc->cenc.default_encrypted_sample;
         } else {
             av_log(mov->fc, AV_LOG_ERROR, "Incorrect number of samples in encryption info\n");
             return AVERROR_INVALIDDATA;
-- 
2.17.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/4] avformat/mov: Avoid cloning encryption info if its unchanged

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 654 bytes --]

On Sat, Jun 18, 2022 at 09:16:34PM +0200, Michael Niedermayer wrote:
> Fixes: OOM
> Fixes: 45834/clusterfuzz-testcase-minimized-ffmpeg_dem_MOV_fuzzer-5419540462305280
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavformat/mov.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)

It appears this issue is still open
so i will apply this

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No snowflake in an avalanche ever feels responsible. -- Voltaire

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".