[FFmpeg-devel] [PATCH 01/13] avformat/flvdec: Use 64bit for sum_flv_tag_size

[FFmpeg-devel] [PATCH 01/13] avformat/flvdec: Use 64bit for sum_flv_tag_size

[Michael Niedermayer]

Fixes: signed integer overflow: 2138820085 + 16130322 cannot be represented in type 'int'
Fixes: 50993/clusterfuzz-testcase-minimized-ffmpeg_dem_LIVE_FLV_fuzzer-6704728165187584

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

diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
index 7f9d7950448..d83edff727c 100644
--- a/libavformat/flvdec.c
+++ b/libavformat/flvdec.c
@@ -66,7 +66,7 @@ typedef struct FLVContext {
     uint8_t resync_buffer[2*RESYNC_BUFFER_SIZE];
 
     int broken_sizes;
-    int sum_flv_tag_size;
+    int64_t sum_flv_tag_size;
 
     int last_keyframe_stream_index;
     int keyframe_count;
@@ -1032,7 +1032,7 @@ retry:
     type = (avio_r8(s->pb) & 0x1F);
     orig_size =
     size = avio_rb24(s->pb);
-    flv->sum_flv_tag_size += size + 11;
+    flv->sum_flv_tag_size += size + 11LL;
     dts  = avio_rb24(s->pb);
     dts |= (unsigned)avio_r8(s->pb) << 24;
     av_log(s, AV_LOG_TRACE, "type:%d, size:%d, last:%d, dts:%"PRId64" pos:%"PRId64"\n", type, size, last, dts, avio_tell(s->pb));
@@ -1332,7 +1332,7 @@ leave:
             !avio_feof(s->pb) &&
             (last != orig_size || !last) && last != flv->sum_flv_tag_size &&
             !flv->broken_sizes) {
-            av_log(s, AV_LOG_ERROR, "Packet mismatch %d %d %d\n", last, orig_size + 11, flv->sum_flv_tag_size);
+            av_log(s, AV_LOG_ERROR, "Packet mismatch %d %d %"PRId64"\n", last, orig_size + 11, flv->sum_flv_tag_size);
             avio_seek(s->pb, pos + 1, SEEK_SET);
             ret = resync(s);
             av_packet_unref(pkt);
-- 
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 02/13] avformat/mxfdec: Check run_in to fit in int and be valid

[Michael Niedermayer]

Fixes: signed integer overflow: 9223372036854775807 - -2146905566 cannot be represented in type 'long'
Fixes: 50993/clusterfuzz-testcase-minimized-ffmpeg_dem_MXF_fuzzer-6570996594769920

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

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index e63e803aa56..da81fea3bc1 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -3681,6 +3681,7 @@ static int mxf_read_header(AVFormatContext *s)
     KLVPacket klv;
     int64_t essence_offset = 0;
     int ret;
+    int64_t run_in;
 
     mxf->last_forward_tell = INT64_MAX;
 
@@ -3690,7 +3691,10 @@ static int mxf_read_header(AVFormatContext *s)
     }
     avio_seek(s->pb, -14, SEEK_CUR);
     mxf->fc = s;
-    mxf->run_in = avio_tell(s->pb);
+    run_in = avio_tell(s->pb);
+    if (run_in < 0 || run_in != (int)run_in)
+        return AVERROR_INVALIDDATA;
+    mxf->run_in = run_in;
 
     mxf_read_random_index_pack(s);
 
-- 
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 02/13] avformat/mxfdec: Check run_in to fit in int and be valid

[Tomas Härdin]

sön 2022-09-18 klockan 19:13 +0200 skrev Michael Niedermayer:
> Fixes: signed integer overflow: 9223372036854775807 - -2146905566
> cannot be represented in type 'long'
> Fixes: 50993/clusterfuzz-testcase-minimized-ffmpeg_dem_MXF_fuzzer-
> 6570996594769920
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavformat/mxfdec.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index e63e803aa56..da81fea3bc1 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -3681,6 +3681,7 @@ static int mxf_read_header(AVFormatContext *s)
>      KLVPacket klv;
>      int64_t essence_offset = 0;
>      int ret;
> +    int64_t run_in;
>  
>      mxf->last_forward_tell = INT64_MAX;
>  
> @@ -3690,7 +3691,10 @@ static int mxf_read_header(AVFormatContext *s)
>      }
>      avio_seek(s->pb, -14, SEEK_CUR);
>      mxf->fc = s;
> -    mxf->run_in = avio_tell(s->pb);
> +    run_in = avio_tell(s->pb);
> +    if (run_in < 0 || run_in != (int)run_in)

run_in > INT_MAX is more clear

It strikes me that run_in is also used in lots of places in the demuxer
without checking for overflow

/Tomas

_______________________________________________
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 02/13] avformat/mxfdec: Check run_in to fit in int and be valid

[Tomas Härdin]

tis 2022-09-20 klockan 13:07 +0200 skrev Tomas Härdin:
> sön 2022-09-18 klockan 19:13 +0200 skrev Michael Niedermayer:
> > Fixes: signed integer overflow: 9223372036854775807 - -2146905566
> > cannot be represented in type 'long'
> > Fixes: 50993/clusterfuzz-testcase-minimized-ffmpeg_dem_MXF_fuzzer-
> > 6570996594769920
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavformat/mxfdec.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > index e63e803aa56..da81fea3bc1 100644
> > --- a/libavformat/mxfdec.c
> > +++ b/libavformat/mxfdec.c
> > @@ -3681,6 +3681,7 @@ static int mxf_read_header(AVFormatContext
> > *s)
> >      KLVPacket klv;
> >      int64_t essence_offset = 0;
> >      int ret;
> > +    int64_t run_in;
> >  
> >      mxf->last_forward_tell = INT64_MAX;
> >  
> > @@ -3690,7 +3691,10 @@ static int mxf_read_header(AVFormatContext
> > *s)
> >      }
> >      avio_seek(s->pb, -14, SEEK_CUR);
> >      mxf->fc = s;
> > -    mxf->run_in = avio_tell(s->pb);
> > +    run_in = avio_tell(s->pb);
> > +    if (run_in < 0 || run_in != (int)run_in)
> 
> run_in > INT_MAX is more clear
> 
> It strikes me that run_in is also used in lots of places in the
> demuxer
> without checking for overflow

I went and checked S377m and the run-in sequence "shall be less than
65536 bytes long". Both the 2004 and 2009 version of the spec agree on
this. So we should reject run_in >= 65536, and mxf_probe() should be
similarly adjusted.

/Tomas

_______________________________________________
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 02/13] avformat/mxfdec: Check run_in to fit in int and be valid

[Michael Niedermayer]

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

On Tue, Sep 20, 2022 at 01:20:00PM +0200, Tomas Härdin wrote:
> tis 2022-09-20 klockan 13:07 +0200 skrev Tomas Härdin:
> > sön 2022-09-18 klockan 19:13 +0200 skrev Michael Niedermayer:
> > > Fixes: signed integer overflow: 9223372036854775807 - -2146905566
> > > cannot be represented in type 'long'
> > > Fixes: 50993/clusterfuzz-testcase-minimized-ffmpeg_dem_MXF_fuzzer-
> > > 6570996594769920
> > > 
> > > Found-by: continuous fuzzing process 
> > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > ---
> > >  libavformat/mxfdec.c | 6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > > index e63e803aa56..da81fea3bc1 100644
> > > --- a/libavformat/mxfdec.c
> > > +++ b/libavformat/mxfdec.c
> > > @@ -3681,6 +3681,7 @@ static int mxf_read_header(AVFormatContext
> > > *s)
> > >      KLVPacket klv;
> > >      int64_t essence_offset = 0;
> > >      int ret;
> > > +    int64_t run_in;
> > >  
> > >      mxf->last_forward_tell = INT64_MAX;
> > >  
> > > @@ -3690,7 +3691,10 @@ static int mxf_read_header(AVFormatContext
> > > *s)
> > >      }
> > >      avio_seek(s->pb, -14, SEEK_CUR);
> > >      mxf->fc = s;
> > > -    mxf->run_in = avio_tell(s->pb);
> > > +    run_in = avio_tell(s->pb);
> > > +    if (run_in < 0 || run_in != (int)run_in)
> > 
> > run_in > INT_MAX is more clear
> > 
> > It strikes me that run_in is also used in lots of places in the
> > demuxer
> > without checking for overflow
> 
> I went and checked S377m and the run-in sequence "shall be less than
> 65536 bytes long". Both the 2004 and 2009 version of the spec agree on
> this. So we should reject run_in >= 65536, and mxf_probe() should be
> similarly adjusted.

ok, will do

thx for checking

i will change the patch by:
@@ -3717,7 +3717,7 @@ static int mxf_read_header(AVFormatContext *s)
     avio_seek(s->pb, -14, SEEK_CUR);
     mxf->fc = s;
     run_in = avio_tell(s->pb);
-    if (run_in < 0 || run_in != (int)run_in)
+    if (run_in < 0 || run_in > 65535)
         return AVERROR_INVALIDDATA;
     mxf->run_in = run_in;
 
@@ -4125,7 +4125,7 @@ static int mxf_read_close(AVFormatContext *s)
 
 static int mxf_probe(const AVProbeData *p) {
     const uint8_t *bufp = p->buf;
-    const uint8_t *end = p->buf + p->buf_size;
+    const uint8_t *end = p->buf + FFMIN(p->buf_size, 65536 + sizeof(mxf_header_partition_pack_key));
 
     if (p->buf_size < sizeof(mxf_header_partition_pack_key))
         return 0;

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

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates

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

Re: [FFmpeg-devel] [PATCH 02/13] avformat/mxfdec: Check run_in to fit in int and be valid

[Tomas Härdin]

ons 2022-09-21 klockan 11:35 +0200 skrev Michael Niedermayer:
> On Tue, Sep 20, 2022 at 01:20:00PM +0200, Tomas Härdin wrote:
> > tis 2022-09-20 klockan 13:07 +0200 skrev Tomas Härdin:
> > > sön 2022-09-18 klockan 19:13 +0200 skrev Michael Niedermayer:
> > > > Fixes: signed integer overflow: 9223372036854775807 - -
> > > > 2146905566
> > > > cannot be represented in type 'long'
> > > > Fixes: 50993/clusterfuzz-testcase-minimized-
> > > > ffmpeg_dem_MXF_fuzzer-
> > > > 6570996594769920
> > > > 
> > > > Found-by: continuous fuzzing process 
> > > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > > ---
> > > >  libavformat/mxfdec.c | 6 +++++-
> > > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > > > index e63e803aa56..da81fea3bc1 100644
> > > > --- a/libavformat/mxfdec.c
> > > > +++ b/libavformat/mxfdec.c
> > > > @@ -3681,6 +3681,7 @@ static int
> > > > mxf_read_header(AVFormatContext
> > > > *s)
> > > >      KLVPacket klv;
> > > >      int64_t essence_offset = 0;
> > > >      int ret;
> > > > +    int64_t run_in;
> > > >  
> > > >      mxf->last_forward_tell = INT64_MAX;
> > > >  
> > > > @@ -3690,7 +3691,10 @@ static int
> > > > mxf_read_header(AVFormatContext
> > > > *s)
> > > >      }
> > > >      avio_seek(s->pb, -14, SEEK_CUR);
> > > >      mxf->fc = s;
> > > > -    mxf->run_in = avio_tell(s->pb);
> > > > +    run_in = avio_tell(s->pb);
> > > > +    if (run_in < 0 || run_in != (int)run_in)
> > > 
> > > run_in > INT_MAX is more clear
> > > 
> > > It strikes me that run_in is also used in lots of places in the
> > > demuxer
> > > without checking for overflow
> > 
> > I went and checked S377m and the run-in sequence "shall be less
> > than
> > 65536 bytes long". Both the 2004 and 2009 version of the spec agree
> > on
> > this. So we should reject run_in >= 65536, and mxf_probe() should
> > be
> > similarly adjusted.
> 
> ok, will do
> 
> thx for checking
> 
> i will change the patch by:
> @@ -3717,7 +3717,7 @@ static int mxf_read_header(AVFormatContext *s)
>      avio_seek(s->pb, -14, SEEK_CUR);
>      mxf->fc = s;
>      run_in = avio_tell(s->pb);
> -    if (run_in < 0 || run_in != (int)run_in)
> +    if (run_in < 0 || run_in > 65535)

Let's avoid magic numbers:
#define RUN_IN_MAX 65535  // S377m-2004 section 5.5 and S377-1-2009
section 6.5

>          return AVERROR_INVALIDDATA;
>      mxf->run_in = run_in;
>  
> @@ -4125,7 +4125,7 @@ static int mxf_read_close(AVFormatContext *s)
>  
>  static int mxf_probe(const AVProbeData *p) {
>      const uint8_t *bufp = p->buf;
> -    const uint8_t *end = p->buf + p->buf_size;
> +    const uint8_t *end = p->buf + FFMIN(p->buf_size, 65536 +
> sizeof(mxf_header_partition_pack_key));

Seems correct. I tested this by prefixing fate-suite/mxf/Meridian-
Apple_ProResProxy-HDR10.mxf with 65535 NUL bytes which worked fine, and
65536 NUL bytes which does not probe as MXF as expected

/Tomas

_______________________________________________
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 02/13] avformat/mxfdec: Check run_in to fit in int and be valid

[Michael Niedermayer]

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

On Wed, Sep 21, 2022 at 03:23:21PM +0200, Tomas Härdin wrote:
> ons 2022-09-21 klockan 11:35 +0200 skrev Michael Niedermayer:
> > On Tue, Sep 20, 2022 at 01:20:00PM +0200, Tomas Härdin wrote:
> > > tis 2022-09-20 klockan 13:07 +0200 skrev Tomas Härdin:
> > > > sön 2022-09-18 klockan 19:13 +0200 skrev Michael Niedermayer:
> > > > > Fixes: signed integer overflow: 9223372036854775807 - -
> > > > > 2146905566
> > > > > cannot be represented in type 'long'
> > > > > Fixes: 50993/clusterfuzz-testcase-minimized-
> > > > > ffmpeg_dem_MXF_fuzzer-
> > > > > 6570996594769920
> > > > > 
> > > > > Found-by: continuous fuzzing process 
> > > > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > > > ---
> > > > >  libavformat/mxfdec.c | 6 +++++-
> > > > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > > > > index e63e803aa56..da81fea3bc1 100644
> > > > > --- a/libavformat/mxfdec.c
> > > > > +++ b/libavformat/mxfdec.c
> > > > > @@ -3681,6 +3681,7 @@ static int
> > > > > mxf_read_header(AVFormatContext
> > > > > *s)
> > > > >      KLVPacket klv;
> > > > >      int64_t essence_offset = 0;
> > > > >      int ret;
> > > > > +    int64_t run_in;
> > > > >  
> > > > >      mxf->last_forward_tell = INT64_MAX;
> > > > >  
> > > > > @@ -3690,7 +3691,10 @@ static int
> > > > > mxf_read_header(AVFormatContext
> > > > > *s)
> > > > >      }
> > > > >      avio_seek(s->pb, -14, SEEK_CUR);
> > > > >      mxf->fc = s;
> > > > > -    mxf->run_in = avio_tell(s->pb);
> > > > > +    run_in = avio_tell(s->pb);
> > > > > +    if (run_in < 0 || run_in != (int)run_in)
> > > > 
> > > > run_in > INT_MAX is more clear
> > > > 
> > > > It strikes me that run_in is also used in lots of places in the
> > > > demuxer
> > > > without checking for overflow
> > > 
> > > I went and checked S377m and the run-in sequence "shall be less
> > > than
> > > 65536 bytes long". Both the 2004 and 2009 version of the spec agree
> > > on
> > > this. So we should reject run_in >= 65536, and mxf_probe() should
> > > be
> > > similarly adjusted.
> > 
> > ok, will do
> > 
> > thx for checking
> > 
> > i will change the patch by:
> > @@ -3717,7 +3717,7 @@ static int mxf_read_header(AVFormatContext *s)
> >      avio_seek(s->pb, -14, SEEK_CUR);
> >      mxf->fc = s;
> >      run_in = avio_tell(s->pb);
> > -    if (run_in < 0 || run_in != (int)run_in)
> > +    if (run_in < 0 || run_in > 65535)
> 
> Let's avoid magic numbers:
> #define RUN_IN_MAX 65535  // S377m-2004 section 5.5 and S377-1-2009
> section 6.5

sure, will use this

thx

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

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle

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

Re: [FFmpeg-devel] [PATCH 02/13] avformat/mxfdec: Check run_in to fit in int and be valid

[Tomas Härdin]

ons 2022-09-21 klockan 11:35 +0200 skrev Michael Niedermayer:
> On Tue, Sep 20, 2022 at 01:20:00PM +0200, Tomas Härdin wrote:
> > tis 2022-09-20 klockan 13:07 +0200 skrev Tomas Härdin:
> > > sön 2022-09-18 klockan 19:13 +0200 skrev Michael Niedermayer:
> > > > Fixes: signed integer overflow: 9223372036854775807 - -
> > > > 2146905566
> > > > cannot be represented in type 'long'
> > > > Fixes: 50993/clusterfuzz-testcase-minimized-
> > > > ffmpeg_dem_MXF_fuzzer-
> > > > 6570996594769920
> > > > 
> > > > Found-by: continuous fuzzing process 
> > > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > > ---
> > > >  libavformat/mxfdec.c | 6 +++++-
> > > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > > > index e63e803aa56..da81fea3bc1 100644
> > > > --- a/libavformat/mxfdec.c
> > > > +++ b/libavformat/mxfdec.c
> > > > @@ -3681,6 +3681,7 @@ static int
> > > > mxf_read_header(AVFormatContext
> > > > *s)
> > > >      KLVPacket klv;
> > > >      int64_t essence_offset = 0;
> > > >      int ret;
> > > > +    int64_t run_in;
> > > >  
> > > >      mxf->last_forward_tell = INT64_MAX;
> > > >  
> > > > @@ -3690,7 +3691,10 @@ static int
> > > > mxf_read_header(AVFormatContext
> > > > *s)
> > > >      }
> > > >      avio_seek(s->pb, -14, SEEK_CUR);
> > > >      mxf->fc = s;
> > > > -    mxf->run_in = avio_tell(s->pb);
> > > > +    run_in = avio_tell(s->pb);
> > > > +    if (run_in < 0 || run_in != (int)run_in)
> > > 
> > > run_in > INT_MAX is more clear
> > > 
> > > It strikes me that run_in is also used in lots of places in the
> > > demuxer
> > > without checking for overflow
> > 
> > I went and checked S377m and the run-in sequence "shall be less
> > than
> > 65536 bytes long". Both the 2004 and 2009 version of the spec agree
> > on
> > this. So we should reject run_in >= 65536, and mxf_probe() should
> > be
> > similarly adjusted.
> 
> ok, will do
> 
> thx for checking
> 
> i will change the patch by:
> @@ -3717,7 +3717,7 @@ static int mxf_read_header(AVFormatContext *s)
>      avio_seek(s->pb, -14, SEEK_CUR);

Oh and also the call to mxf_read_sync() could be supplied with a
maximum number of bytes to read, allowing the code to bail out faster

/Tomas

_______________________________________________
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 02/13] avformat/mxfdec: Check run_in to fit in int and be valid

[Michael Niedermayer]

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

On Wed, Sep 21, 2022 at 03:25:33PM +0200, Tomas Härdin wrote:
> ons 2022-09-21 klockan 11:35 +0200 skrev Michael Niedermayer:
> > On Tue, Sep 20, 2022 at 01:20:00PM +0200, Tomas Härdin wrote:
> > > tis 2022-09-20 klockan 13:07 +0200 skrev Tomas Härdin:
> > > > sön 2022-09-18 klockan 19:13 +0200 skrev Michael Niedermayer:
> > > > > Fixes: signed integer overflow: 9223372036854775807 - -
> > > > > 2146905566
> > > > > cannot be represented in type 'long'
> > > > > Fixes: 50993/clusterfuzz-testcase-minimized-
> > > > > ffmpeg_dem_MXF_fuzzer-
> > > > > 6570996594769920
> > > > > 
> > > > > Found-by: continuous fuzzing process 
> > > > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > > > ---
> > > > >  libavformat/mxfdec.c | 6 +++++-
> > > > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > > > > index e63e803aa56..da81fea3bc1 100644
> > > > > --- a/libavformat/mxfdec.c
> > > > > +++ b/libavformat/mxfdec.c
> > > > > @@ -3681,6 +3681,7 @@ static int
> > > > > mxf_read_header(AVFormatContext
> > > > > *s)
> > > > >      KLVPacket klv;
> > > > >      int64_t essence_offset = 0;
> > > > >      int ret;
> > > > > +    int64_t run_in;
> > > > >  
> > > > >      mxf->last_forward_tell = INT64_MAX;
> > > > >  
> > > > > @@ -3690,7 +3691,10 @@ static int
> > > > > mxf_read_header(AVFormatContext
> > > > > *s)
> > > > >      }
> > > > >      avio_seek(s->pb, -14, SEEK_CUR);
> > > > >      mxf->fc = s;
> > > > > -    mxf->run_in = avio_tell(s->pb);
> > > > > +    run_in = avio_tell(s->pb);
> > > > > +    if (run_in < 0 || run_in != (int)run_in)
> > > > 
> > > > run_in > INT_MAX is more clear
> > > > 
> > > > It strikes me that run_in is also used in lots of places in the
> > > > demuxer
> > > > without checking for overflow
> > > 
> > > I went and checked S377m and the run-in sequence "shall be less
> > > than
> > > 65536 bytes long". Both the 2004 and 2009 version of the spec agree
> > > on
> > > this. So we should reject run_in >= 65536, and mxf_probe() should
> > > be
> > > similarly adjusted.
> > 
> > ok, will do
> > 
> > thx for checking
> > 
> > i will change the patch by:
> > @@ -3717,7 +3717,7 @@ static int mxf_read_header(AVFormatContext *s)
> >      avio_seek(s->pb, -14, SEEK_CUR);
> 
> Oh and also the call to mxf_read_sync() could be supplied with a
> maximum number of bytes to read, allowing the code to bail out faster

I agree, but that should be seperate from the initial bugfix

thx

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

Re: [FFmpeg-devel] [PATCH 02/13] avformat/mxfdec: Check run_in to fit in int and be valid

[Marton Balint]

On Wed, 21 Sep 2022, Tomas Härdin wrote:

> ons 2022-09-21 klockan 11:35 +0200 skrev Michael Niedermayer:
>> On Tue, Sep 20, 2022 at 01:20:00PM +0200, Tomas Härdin wrote:
>> > tis 2022-09-20 klockan 13:07 +0200 skrev Tomas Härdin:
>> > > sön 2022-09-18 klockan 19:13 +0200 skrev Michael Niedermayer:
>> > > > Fixes: signed integer overflow: 9223372036854775807 - -
>> > > > 2146905566
>> > > > cannot be represented in type 'long'
>> > > > Fixes: 50993/clusterfuzz-testcase-minimized-
>> > > > ffmpeg_dem_MXF_fuzzer-
>> > > > 6570996594769920
>> > > > 
>> > > > Found-by: continuous fuzzing process 
>> > > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>> > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>> > > > ---
>> > > >  libavformat/mxfdec.c | 6 +++++-
>> > > >  1 file changed, 5 insertions(+), 1 deletion(-)
>> > > > 
>> > > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
>> > > > index e63e803aa56..da81fea3bc1 100644
>> > > > --- a/libavformat/mxfdec.c
>> > > > +++ b/libavformat/mxfdec.c
>> > > > @@ -3681,6 +3681,7 @@ static int
>> > > > mxf_read_header(AVFormatContext
>> > > > *s)
>> > > >      KLVPacket klv;
>> > > >      int64_t essence_offset = 0;
>> > > >      int ret;
>> > > > +    int64_t run_in;
>> > > >  
>> > > >      mxf->last_forward_tell = INT64_MAX;
>> > > >  
>> > > > @@ -3690,7 +3691,10 @@ static int
>> > > > mxf_read_header(AVFormatContext
>> > > > *s)
>> > > >      }
>> > > >      avio_seek(s->pb, -14, SEEK_CUR);
>> > > >      mxf->fc = s;
>> > > > -    mxf->run_in = avio_tell(s->pb);
>> > > > +    run_in = avio_tell(s->pb);
>> > > > +    if (run_in < 0 || run_in != (int)run_in)
>> > > 
>> > > run_in > INT_MAX is more clear
>> > > 
>> > > It strikes me that run_in is also used in lots of places in the
>> > > demuxer
>> > > without checking for overflow
>> > 
>> > I went and checked S377m and the run-in sequence "shall be less
>> > than
>> > 65536 bytes long". Both the 2004 and 2009 version of the spec agree
>> > on
>> > this. So we should reject run_in >= 65536, and mxf_probe() should
>> > be
>> > similarly adjusted.
>> 
>> ok, will do
>> 
>> thx for checking
>> 
>> i will change the patch by:
>> @@ -3717,7 +3717,7 @@ static int mxf_read_header(AVFormatContext *s)
>>      avio_seek(s->pb, -14, SEEK_CUR);
>
> Oh and also the call to mxf_read_sync() could be supplied with a
> maximum number of bytes to read, allowing the code to bail out faster

Yeah, I wanted to suggest this as well. And please allow 65536-byte 
run-in, even if that is not strictly allowed by the current standard, 
because the MXF book has no problem with that:

(run-in is) "...a sequence of up to 65536 bytes at the front of the file 
that precedes the first partition pack..."

Thanks,
Marton
_______________________________________________
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 02/13] avformat/mxfdec: Check run_in to fit in int and be valid

[Tomas Härdin]

ons 2022-09-21 klockan 20:56 +0200 skrev Marton Balint:
> 
> 
> On Wed, 21 Sep 2022, Tomas Härdin wrote:
> 
> > ons 2022-09-21 klockan 11:35 +0200 skrev Michael Niedermayer:
> > > On Tue, Sep 20, 2022 at 01:20:00PM +0200, Tomas Härdin wrote:
> > > > tis 2022-09-20 klockan 13:07 +0200 skrev Tomas Härdin:
> > > > > sön 2022-09-18 klockan 19:13 +0200 skrev Michael Niedermayer:
> > > > > > Fixes: signed integer overflow: 9223372036854775807 - -
> > > > > > 2146905566
> > > > > > cannot be represented in type 'long'
> > > > > > Fixes: 50993/clusterfuzz-testcase-minimized-
> > > > > > ffmpeg_dem_MXF_fuzzer-
> > > > > > 6570996594769920
> > > > > > 
> > > > > > Found-by: continuous fuzzing process 
> > > > > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > > > > ---
> > > > > >  libavformat/mxfdec.c | 6 +++++-
> > > > > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > > > > > index e63e803aa56..da81fea3bc1 100644
> > > > > > --- a/libavformat/mxfdec.c
> > > > > > +++ b/libavformat/mxfdec.c
> > > > > > @@ -3681,6 +3681,7 @@ static int
> > > > > > mxf_read_header(AVFormatContext
> > > > > > *s)
> > > > > >      KLVPacket klv;
> > > > > >      int64_t essence_offset = 0;
> > > > > >      int ret;
> > > > > > +    int64_t run_in;
> > > > > >  
> > > > > >      mxf->last_forward_tell = INT64_MAX;
> > > > > >  
> > > > > > @@ -3690,7 +3691,10 @@ static int
> > > > > > mxf_read_header(AVFormatContext
> > > > > > *s)
> > > > > >      }
> > > > > >      avio_seek(s->pb, -14, SEEK_CUR);
> > > > > >      mxf->fc = s;
> > > > > > -    mxf->run_in = avio_tell(s->pb);
> > > > > > +    run_in = avio_tell(s->pb);
> > > > > > +    if (run_in < 0 || run_in != (int)run_in)
> > > > > 
> > > > > run_in > INT_MAX is more clear
> > > > > 
> > > > > It strikes me that run_in is also used in lots of places in
> > > > > the
> > > > > demuxer
> > > > > without checking for overflow
> > > > 
> > > > I went and checked S377m and the run-in sequence "shall be less
> > > > than
> > > > 65536 bytes long". Both the 2004 and 2009 version of the spec
> > > > agree
> > > > on
> > > > this. So we should reject run_in >= 65536, and mxf_probe()
> > > > should
> > > > be
> > > > similarly adjusted.
> > > 
> > > ok, will do
> > > 
> > > thx for checking
> > > 
> > > i will change the patch by:
> > > @@ -3717,7 +3717,7 @@ static int mxf_read_header(AVFormatContext
> > > *s)
> > >      avio_seek(s->pb, -14, SEEK_CUR);
> > 
> > Oh and also the call to mxf_read_sync() could be supplied with a
> > maximum number of bytes to read, allowing the code to bail out
> > faster
> 
> Yeah, I wanted to suggest this as well. And please allow 65536-byte 
> run-in, even if that is not strictly allowed by the current standard,
> because the MXF book has no problem with that:
> 
> (run-in is) "...a sequence of up to 65536 bytes at the front of the
> file 
> that precedes the first partition pack..."

The MXF Book is not S377m. It also has other errors in it IIRC

/Tomas

_______________________________________________
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 03/13] avformat/nutdec: Check fields

[Michael Niedermayer]

Fixes: signed integer overflow: -2147483648 - 1 cannot be represented in type 'int'
Fixes: 50993/clusterfuzz-testcase-minimized-ffmpeg_dem_NUT_fuzzer-6566001610719232

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

diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
index 8cc56615ad7..24dedc47582 100644
--- a/libavformat/nutdec.c
+++ b/libavformat/nutdec.c
@@ -245,6 +245,11 @@ static int decode_main_header(NUTContext *nut)
     for (i = 0; i < 256;) {
         int tmp_flags  = ffio_read_varlen(bc);
         int tmp_fields = ffio_read_varlen(bc);
+        if (tmp_fields < 0) {
+            av_log(s, AV_LOG_ERROR, "fields %d is invalid\n", tmp_fields);
+            ret = AVERROR_INVALIDDATA;
+            goto fail;
+        }
 
         if (tmp_fields > 0)
             tmp_pts = get_s(bc);
-- 
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 04/13] avformat/rmdec: check tag_size

[Michael Niedermayer]

Fixes: signed integer overflow: -2147483648 - 8 cannot be represented in type 'int'
Fixes: 50993/clusterfuzz-testcase-minimized-ffmpeg_dem_RM_fuzzer-6598073725353984

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

diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
index 881d7002add..0f1534b5820 100644
--- a/libavformat/rmdec.c
+++ b/libavformat/rmdec.c
@@ -563,6 +563,8 @@ static int rm_read_header(AVFormatContext *s)
     }
 
     tag_size = avio_rb32(pb);
+    if (tag_size < 0)
+        return AVERROR_INVALIDDATA;
     avio_skip(pb, tag_size - 8);
 
     for(;;) {
-- 
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 05/13] avformat/sbgdec: clamp end_ts

[Michael Niedermayer]

Fixes: signed integer overflow: 9223372036851135042 + 15666854 cannot be represented in type 'long'
Fixes: 50993/clusterfuzz-testcase-minimized-ffmpeg_dem_SBG_fuzzer-6573717339111424

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

diff --git a/libavformat/sbgdec.c b/libavformat/sbgdec.c
index 8a6d6790566..4cd12347e7b 100644
--- a/libavformat/sbgdec.c
+++ b/libavformat/sbgdec.c
@@ -1478,7 +1478,7 @@ static int sbg_read_packet(AVFormatContext *avf, AVPacket *packet)
     int ret;
 
     ts = ffstream(avf->streams[0])->cur_dts;
-    end_ts = ts + avf->streams[0]->codecpar->frame_size;
+    end_ts = av_sat_add64(ts, avf->streams[0]->codecpar->frame_size);
     if (avf->streams[0]->duration != AV_NOPTS_VALUE)
         end_ts = FFMIN(avf->streams[0]->start_time + avf->streams[0]->duration,
                        end_ts);
-- 
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 06/13] avformat/sbgdec: Check ts_int in genrate_intervals

[Michael Niedermayer]

There is probably a better place to check for this, but better
here than nowhere

Fixes: signed integer overflow: -9223372036824775808 - 86400000000 cannot be represented in type 'long'
Fixes: 50993/clusterfuzz-testcase-minimized-ffmpeg_dem_SBG_fuzzer-6601162580688896

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

diff --git a/libavformat/sbgdec.c b/libavformat/sbgdec.c
index 4cd12347e7b..5edb9664cc3 100644
--- a/libavformat/sbgdec.c
+++ b/libavformat/sbgdec.c
@@ -1317,6 +1317,8 @@ static int generate_intervals(void *log, struct sbg_script *s, int sample_rate,
 
     /* Pseudo event before the first one */
     ev0 = s->events[s->nb_events - 1];
+    if (av_sat_sub64(ev0.ts_int, period) != (uint64_t)ev0.ts_int - period)
+        return AVERROR_INVALIDDATA;
     ev0.ts_int   -= period;
     ev0.ts_trans -= period;
     ev0.ts_next  -= period;
-- 
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 07/13] avformat/sdsdec: Use av_rescale() to avoid intermediate overflow in duration calculation

[Michael Niedermayer]

Fixes: signed integer overflow: 72128794995445727 * 240 cannot be represented in type 'long'
Fixes: 50993/clusterfuzz-testcase-minimized-ffmpeg_dem_SDS_fuzzer-6628185583779840

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

diff --git a/libavformat/sdsdec.c b/libavformat/sdsdec.c
index f98096dca98..d296500beca 100644
--- a/libavformat/sdsdec.c
+++ b/libavformat/sdsdec.c
@@ -112,7 +112,7 @@ static int sds_read_header(AVFormatContext *ctx)
     st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO;
     st->codecpar->ch_layout.nb_channels = 1;
     st->codecpar->sample_rate = sample_period ? 1000000000 / sample_period : 16000;
-    st->duration = (avio_size(pb) - 21) / (127) * s->size / 4;
+    st->duration = av_rescale((avio_size(pb) - 21) / 127,  s->size, 4);
 
     avpriv_set_pts_info(st, 64, 1, st->codecpar->sample_rate);
 
-- 
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 08/13] avformat/xwma: Use av_rescale() for duration computation

[Michael Niedermayer]

Fixes: signed integer overflow: 34242363648 * 538976288 cannot be represented in type 'long'
Fixes: 50993/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6577923913547776

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

diff --git a/libavformat/xwma.c b/libavformat/xwma.c
index c16ff1be634..12689f37fd7 100644
--- a/libavformat/xwma.c
+++ b/libavformat/xwma.c
@@ -278,7 +278,7 @@ static int xwma_read_header(AVFormatContext *s)
          * the total duration using the average bits per sample and the
          * total data length.
          */
-        st->duration = (size<<3) * st->codecpar->sample_rate / st->codecpar->bit_rate;
+        st->duration = av_rescale((size<<3), st->codecpar->sample_rate, st->codecpar->bit_rate);
     }
 
 fail:
-- 
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 09/13] avformat/rpl: Use 64bit for duration computation

[Michael Niedermayer]

Fixes: signed integer overflow: 24709512 * 88 cannot be represented in type 'int'
Fixes: 50993/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6737973728641024

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

diff --git a/libavformat/rpl.c b/libavformat/rpl.c
index d025589bfc3..3ef6fda3862 100644
--- a/libavformat/rpl.c
+++ b/libavformat/rpl.c
@@ -279,7 +279,7 @@ static int rpl_read_header(AVFormatContext *s)
     error |= read_line(pb, line, sizeof(line));  // size of "helpful" sprite
     if (vst) {
         error |= read_line(pb, line, sizeof(line));  // offset to key frame list
-        vst->duration = number_of_chunks * rpl->frames_per_chunk;
+        vst->duration = number_of_chunks * (int64_t)rpl->frames_per_chunk;
     }
 
     // Read the index
-- 
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 10/13] avformat/vividas: Check packet size

[Michael Niedermayer]

Fixes: signed integer overflow: 119760682 - -2084600173 cannot be represented in type 'int'
Fixes: 50993/clusterfuzz-testcase-minimized-ffmpeg_dem_VIVIDAS_fuzzer-6745781167587328

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

diff --git a/libavformat/vividas.c b/libavformat/vividas.c
index e9954f73ed0..22f61db7576 100644
--- a/libavformat/vividas.c
+++ b/libavformat/vividas.c
@@ -643,7 +643,9 @@ static int viv_read_packet(AVFormatContext *s,
 
     if (viv->current_audio_subpacket < viv->n_audio_subpackets) {
         AVStream *astream;
-        int size = viv->audio_subpackets[viv->current_audio_subpacket+1].start - viv->audio_subpackets[viv->current_audio_subpacket].start;
+        int64_t size = viv->audio_subpackets[viv->current_audio_subpacket+1].start - (int64_t)viv->audio_subpackets[viv->current_audio_subpacket].start;
+        if (size < 0 || size != (int)size)
+            return AVERROR_INVALIDDATA;
 
         pb = viv->sb_pb;
         ret = av_get_packet(pb, pkt, size);
-- 
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 10/13] avformat/vividas: Check packet size

[Anton Khirnov]

Quoting Michael Niedermayer (2022-09-18 19:14:07)
> Fixes: signed integer overflow: 119760682 - -2084600173 cannot be represented in type 'int'
> Fixes: 50993/clusterfuzz-testcase-minimized-ffmpeg_dem_VIVIDAS_fuzzer-6745781167587328
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavformat/vividas.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/vividas.c b/libavformat/vividas.c
> index e9954f73ed0..22f61db7576 100644
> --- a/libavformat/vividas.c
> +++ b/libavformat/vividas.c
> @@ -643,7 +643,9 @@ static int viv_read_packet(AVFormatContext *s,
>  
>      if (viv->current_audio_subpacket < viv->n_audio_subpackets) {
>          AVStream *astream;
> -        int size = viv->audio_subpackets[viv->current_audio_subpacket+1].start - viv->audio_subpackets[viv->current_audio_subpacket].start;
> +        int64_t size = viv->audio_subpackets[viv->current_audio_subpacket+1].start - (int64_t)viv->audio_subpackets[viv->current_audio_subpacket].start;
> +        if (size < 0 || size != (int)size)
> +            return AVERROR_INVALIDDATA;

These values should be checked in the loop where they are set.

-- 
Anton Khirnov
_______________________________________________
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 10/13] avformat/vividas: Check packet size

[Michael Niedermayer]

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

On Thu, Sep 22, 2022 at 03:18:20PM +0200, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2022-09-18 19:14:07)
> > Fixes: signed integer overflow: 119760682 - -2084600173 cannot be represented in type 'int'
> > Fixes: 50993/clusterfuzz-testcase-minimized-ffmpeg_dem_VIVIDAS_fuzzer-6745781167587328
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavformat/vividas.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavformat/vividas.c b/libavformat/vividas.c
> > index e9954f73ed0..22f61db7576 100644
> > --- a/libavformat/vividas.c
> > +++ b/libavformat/vividas.c
> > @@ -643,7 +643,9 @@ static int viv_read_packet(AVFormatContext *s,
> >  
> >      if (viv->current_audio_subpacket < viv->n_audio_subpackets) {
> >          AVStream *astream;
> > -        int size = viv->audio_subpackets[viv->current_audio_subpacket+1].start - viv->audio_subpackets[viv->current_audio_subpacket].start;
> > +        int64_t size = viv->audio_subpackets[viv->current_audio_subpacket+1].start - (int64_t)viv->audio_subpackets[viv->current_audio_subpacket].start;
> > +        if (size < 0 || size != (int)size)
> > +            return AVERROR_INVALIDDATA;
> 
> These values should be checked in the loop where they are set.

ok but then we fail before the actual problem is encountered
i will send a patch doing that

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes

[-- 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 11/13] avformat/spdifdec: Use 64bit to compute bit rate

[Michael Niedermayer]

Fixes: signed integer overflow: 32 * 553590816 cannot be represented in type 'int'
Fixes: 50993/clusterfuzz-testcase-minimized-ffmpeg_dem_WAV_fuzzer-6564974517944320

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

diff --git a/libavformat/spdifdec.c b/libavformat/spdifdec.c
index 2af75ca9dbd..672133581a8 100644
--- a/libavformat/spdifdec.c
+++ b/libavformat/spdifdec.c
@@ -226,7 +226,7 @@ int ff_spdif_read_packet(AVFormatContext *s, AVPacket *pkt)
     if (!s->bit_rate && s->streams[0]->codecpar->sample_rate)
         /* stream bitrate matches 16-bit stereo PCM bitrate for currently
            supported codecs */
-        s->bit_rate = 2 * 16 * s->streams[0]->codecpar->sample_rate;
+        s->bit_rate = 2 * 16LL * s->streams[0]->codecpar->sample_rate;
 
     return 0;
 }
-- 
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 12/13] avformat/matroskadec: Error out if a timestamp is beyond duration

[Michael Niedermayer]

Maybe timestamp / duration validity should be checked earlier

Fixes: 50993/clusterfuzz-testcase-minimized-ffmpeg_dem_WEBM_DASH_MANIFEST_fuzzer-6586894739177472
Fixes: signed integer overflow: 0 - -9223372036854775808 cannot be represented in type 'long'

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

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 16a3e936110..8b079e11104 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -4009,7 +4009,8 @@ typedef struct {
 
 /* This function searches all the Cues and returns the CueDesc corresponding to
  * the timestamp ts. Returned CueDesc will be such that start_time_ns <= ts <
- * end_time_ns. All 4 fields will be set to -1 if ts >= file's duration.
+ * end_time_ns. All 4 fields will be set to -1 if ts >= file's duration or
+ * if an error occurred.
  */
 static CueDesc get_cue_desc(AVFormatContext *s, int64_t ts, int64_t cues_start) {
     MatroskaDemuxContext *matroska = s->priv_data;
@@ -4028,6 +4029,8 @@ static CueDesc get_cue_desc(AVFormatContext *s, int64_t ts, int64_t cues_start)
         }
     }
     --i;
+    if (index_entries[i].timestamp > matroska->duration)
+        return (CueDesc) {-1, -1, -1, -1};
     cue_desc.start_time_ns = index_entries[i].timestamp * matroska->time_scale;
     cue_desc.start_offset = index_entries[i].pos - matroska->segment_start;
     if (i != nb_index_entries - 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 13/13] avformat/westwood_vqa: Check chunk size

[Michael Niedermayer]

Fixes: signed integer overflow: 2147483647 + 1 cannot be represented in type 'int'
Fixes: 50993/clusterfuzz-testcase-minimized-ffmpeg_dem_WSVQA_fuzzer-6593408795279360

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

diff --git a/libavformat/westwood_vqa.c b/libavformat/westwood_vqa.c
index e3d2e2668c4..bd8df5e0b34 100644
--- a/libavformat/westwood_vqa.c
+++ b/libavformat/westwood_vqa.c
@@ -274,6 +274,8 @@ static int wsvqa_read_packet(AVFormatContext *s,
                         return AVERROR(EIO);
 
                     /* the decoder expects chunks to be 16-bit aligned */
+                    if (wsvqa->vqfl_chunk_size == INT_MAX)
+                        return AVERROR_INVALIDDATA;
                     if (wsvqa->vqfl_chunk_size % 2 == 1)
                         wsvqa->vqfl_chunk_size++;
 
-- 
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 13/13] avformat/westwood_vqa: Check chunk size

[Anton Khirnov]

Quoting Michael Niedermayer (2022-09-18 19:14:10)
> Fixes: signed integer overflow: 2147483647 + 1 cannot be represented in type 'int'
> Fixes: 50993/clusterfuzz-testcase-minimized-ffmpeg_dem_WSVQA_fuzzer-6593408795279360
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavformat/westwood_vqa.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/libavformat/westwood_vqa.c b/libavformat/westwood_vqa.c
> index e3d2e2668c4..bd8df5e0b34 100644
> --- a/libavformat/westwood_vqa.c
> +++ b/libavformat/westwood_vqa.c
> @@ -274,6 +274,8 @@ static int wsvqa_read_packet(AVFormatContext *s,
>                          return AVERROR(EIO);
>  
>                      /* the decoder expects chunks to be 16-bit aligned */
> +                    if (wsvqa->vqfl_chunk_size == INT_MAX)
> +                        return AVERROR_INVALIDDATA;

IIUC this can only happen after the
  wsvqa->vqfl_chunk_size > 3 * (1 << 20)
check above was triggered. Then it makes more sense to change that code
to check chunk_size before setting wsvqa->vqfl_chunk_size.

-- 
Anton Khirnov
_______________________________________________
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 13/13] avformat/westwood_vqa: Check chunk size

[Michael Niedermayer]

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

On Thu, Sep 22, 2022 at 03:01:05PM +0200, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2022-09-18 19:14:10)
> > Fixes: signed integer overflow: 2147483647 + 1 cannot be represented in type 'int'
> > Fixes: 50993/clusterfuzz-testcase-minimized-ffmpeg_dem_WSVQA_fuzzer-6593408795279360
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavformat/westwood_vqa.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/libavformat/westwood_vqa.c b/libavformat/westwood_vqa.c
> > index e3d2e2668c4..bd8df5e0b34 100644
> > --- a/libavformat/westwood_vqa.c
> > +++ b/libavformat/westwood_vqa.c
> > @@ -274,6 +274,8 @@ static int wsvqa_read_packet(AVFormatContext *s,
> >                          return AVERROR(EIO);
> >  
> >                      /* the decoder expects chunks to be 16-bit aligned */
> > +                    if (wsvqa->vqfl_chunk_size == INT_MAX)
> > +                        return AVERROR_INVALIDDATA;
> 
> IIUC this can only happen after the
>   wsvqa->vqfl_chunk_size > 3 * (1 << 20)
> check above was triggered. Then it makes more sense to change that code
> to check chunk_size before setting wsvqa->vqfl_chunk_size.

i will post a new patch

thx

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

It is what and why we do it that matters, not just one of them.

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

Re: [FFmpeg-devel] [PATCH 01/13] avformat/flvdec: Use 64bit for sum_flv_tag_size

[Michael Niedermayer]

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

On Sun, Sep 18, 2022 at 07:13:58PM +0200, Michael Niedermayer wrote:
> Fixes: signed integer overflow: 2138820085 + 16130322 cannot be represented in type 'int'
> Fixes: 50993/clusterfuzz-testcase-minimized-ffmpeg_dem_LIVE_FLV_fuzzer-6704728165187584
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavformat/flvdec.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

i intend to apply this patchset (minus anything with objections) soon

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

Those who would give up essential Liberty, to purchase a little
temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin

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