[FFmpeg-devel] [PATCH 1/3] avcodec/osq: Implement flush()

[FFmpeg-devel] [PATCH 1/3] avcodec/osq: Implement flush()

[Michael Niedermayer]

Fixes: out of array access
Fixes: 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6227491892887552
Fixes: 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6268561729126400
Fixes: 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6414805046788096
Fixes: 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6538151088488448
Fixes: 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6608131540779008

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

diff --git a/libavcodec/osq.c b/libavcodec/osq.c
index 435f3ea7f69..abe15c97f18 100644
--- a/libavcodec/osq.c
+++ b/libavcodec/osq.c
@@ -61,6 +61,15 @@ typedef struct OSQContext {
     int pkt_offset;
 } OSQContext;
 
+static void osq_flush(AVCodecContext *avctx)
+{
+    OSQContext *s = avctx->priv_data;
+
+    s->bitstream_size = 0;
+    s->pkt_offset = 0;
+    av_packet_unref(s->pkt);
+}
+
 static av_cold int osq_close(AVCodecContext *avctx)
 {
     OSQContext *s = avctx->priv_data;
@@ -478,4 +487,5 @@ const FFCodec ff_osq_decoder = {
                                                         AV_SAMPLE_FMT_S16P,
                                                         AV_SAMPLE_FMT_S32P,
                                                         AV_SAMPLE_FMT_NONE },
+    .flush            = osq_flush,
 };
-- 
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/3] avcodec/osq: avoid several signed integer overflows

[Michael Niedermayer]

Fixes: signed integer overflow: 178459578 + 2009763270 cannot be represented in type 'int'
Fixes: 62285/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-5013423686287360

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

diff --git a/libavcodec/osq.c b/libavcodec/osq.c
index abe15c97f18..f2771c46eb5 100644
--- a/libavcodec/osq.c
+++ b/libavcodec/osq.c
@@ -222,8 +222,8 @@ static int osq_channel_parameters(AVCodecContext *avctx, int ch)
 #define C (-3)
 #define D (-4)
 #define E (-5)
-#define P2 ((dst[A] + dst[A]) - dst[B])
-#define P3 ((dst[A] - dst[B]) * 3 + dst[C])
+#define P2 (((unsigned)dst[A] + dst[A]) - dst[B])
+#define P3 (((unsigned)dst[A] - dst[B]) * 3 + dst[C])
 
 static int do_decode(AVCodecContext *avctx, AVFrame *frame, int decorrelate, int downsample)
 {
@@ -273,10 +273,10 @@ static int do_decode(AVCodecContext *avctx, AVFrame *frame, int decorrelate, int
             case 0:
                 break;
             case 1:
-                dst[n] += dst[A];
+                dst[n] += (unsigned)dst[A];
                 break;
             case 2:
-                dst[n] += dst[A] + p;
+                dst[n] += (unsigned)dst[A] + p;
                 break;
             case 3:
                 dst[n] += P2;
@@ -291,28 +291,28 @@ static int do_decode(AVCodecContext *avctx, AVFrame *frame, int decorrelate, int
                 dst[n] += P3 + p;
                 break;
             case 7:
-                dst[n] += (P2 + P3) / 2 + p;
+                dst[n] += (int)(P2 + P3) / 2 + (unsigned)p;
                 break;
             case 8:
-                dst[n] += (P2 + P3) / 2;
+                dst[n] += (int)(P2 + P3) / 2;
                 break;
             case 9:
-                dst[n] += (P2 * 2 + P3) / 3 + p;
+                dst[n] += (int)(P2 * 2 + P3) / 3 + (unsigned)p;
                 break;
             case 10:
-                dst[n] += (P2 + P3 * 2) / 3 + p;
+                dst[n] += (int)(P2 + P3 * 2) / 3 + (unsigned)p;
                 break;
             case 11:
-                dst[n] += (dst[A] + dst[B]) / 2;
+                dst[n] += (int)((unsigned)dst[A] + dst[B]) / 2;
                 break;
             case 12:
-                dst[n] += dst[B];
+                dst[n] += (unsigned)dst[B];
                 break;
             case 13:
-                dst[n] += (dst[D] + dst[B]) / 2;
+                dst[n] += (int)(unsigned)(dst[D] + dst[B]) / 2;
                 break;
             case 14:
-                dst[n] += (P2 + dst[A]) / 2 + p;
+                dst[n] += (int)((unsigned)P2 + dst[A]) / 2 + (unsigned)p;
                 break;
             default:
                 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 2/3] avcodec/osq: avoid several signed integer overflows

[James Almer]

On 12/26/2023 1:37 PM, Michael Niedermayer wrote:
> Fixes: signed integer overflow: 178459578 + 2009763270 cannot be represented in type 'int'
> Fixes: 62285/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-5013423686287360
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>   libavcodec/osq.c | 24 ++++++++++++------------
>   1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/libavcodec/osq.c b/libavcodec/osq.c
> index abe15c97f18..f2771c46eb5 100644
> --- a/libavcodec/osq.c
> +++ b/libavcodec/osq.c
> @@ -222,8 +222,8 @@ static int osq_channel_parameters(AVCodecContext *avctx, int ch)
>   #define C (-3)
>   #define D (-4)
>   #define E (-5)
> -#define P2 ((dst[A] + dst[A]) - dst[B])
> -#define P3 ((dst[A] - dst[B]) * 3 + dst[C])
> +#define P2 (((unsigned)dst[A] + dst[A]) - dst[B])
> +#define P3 (((unsigned)dst[A] - dst[B]) * 3 + dst[C])
>   
>   static int do_decode(AVCodecContext *avctx, AVFrame *frame, int decorrelate, int downsample)
>   {
> @@ -273,10 +273,10 @@ static int do_decode(AVCodecContext *avctx, AVFrame *frame, int decorrelate, int
>               case 0:
>                   break;
>               case 1:
> -                dst[n] += dst[A];
> +                dst[n] += (unsigned)dst[A];
>                   break;
>               case 2:
> -                dst[n] += dst[A] + p;
> +                dst[n] += (unsigned)dst[A] + p;
>                   break;
>               case 3:
>                   dst[n] += P2;
> @@ -291,28 +291,28 @@ static int do_decode(AVCodecContext *avctx, AVFrame *frame, int decorrelate, int
>                   dst[n] += P3 + p;
>                   break;
>               case 7:
> -                dst[n] += (P2 + P3) / 2 + p;
> +                dst[n] += (int)(P2 + P3) / 2 + (unsigned)p;

Would 2U work for this? It's shorted and more readable that casts 
everywhere. Same for most cases below.

>                   break;
>               case 8:
> -                dst[n] += (P2 + P3) / 2;
> +                dst[n] += (int)(P2 + P3) / 2;
>                   break;
>               case 9:
> -                dst[n] += (P2 * 2 + P3) / 3 + p;
> +                dst[n] += (int)(P2 * 2 + P3) / 3 + (unsigned)p;
>                   break;
>               case 10:
> -                dst[n] += (P2 + P3 * 2) / 3 + p;
> +                dst[n] += (int)(P2 + P3 * 2) / 3 + (unsigned)p;
>                   break;
>               case 11:
> -                dst[n] += (dst[A] + dst[B]) / 2;
> +                dst[n] += (int)((unsigned)dst[A] + dst[B]) / 2;
>                   break;
>               case 12:
> -                dst[n] += dst[B];
> +                dst[n] += (unsigned)dst[B];
>                   break;
>               case 13:
> -                dst[n] += (dst[D] + dst[B]) / 2;
> +                dst[n] += (int)(unsigned)(dst[D] + dst[B]) / 2;
>                   break;
>               case 14:
> -                dst[n] += (P2 + dst[A]) / 2 + p;
> +                dst[n] += (int)((unsigned)P2 + dst[A]) / 2 + (unsigned)p;
>                   break;
>               default:
>                   return AVERROR_INVALIDDATA;
_______________________________________________
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/3] avcodec/osq: avoid several signed integer overflows

[Michael Niedermayer]

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

On Tue, Dec 26, 2023 at 01:56:31PM -0300, James Almer wrote:
> On 12/26/2023 1:37 PM, Michael Niedermayer wrote:
> > Fixes: signed integer overflow: 178459578 + 2009763270 cannot be represented in type 'int'
> > Fixes: 62285/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-5013423686287360
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >   libavcodec/osq.c | 24 ++++++++++++------------
> >   1 file changed, 12 insertions(+), 12 deletions(-)
> > 
> > diff --git a/libavcodec/osq.c b/libavcodec/osq.c
> > index abe15c97f18..f2771c46eb5 100644
> > --- a/libavcodec/osq.c
> > +++ b/libavcodec/osq.c
> > @@ -222,8 +222,8 @@ static int osq_channel_parameters(AVCodecContext *avctx, int ch)
> >   #define C (-3)
> >   #define D (-4)
> >   #define E (-5)
> > -#define P2 ((dst[A] + dst[A]) - dst[B])
> > -#define P3 ((dst[A] - dst[B]) * 3 + dst[C])
> > +#define P2 (((unsigned)dst[A] + dst[A]) - dst[B])
> > +#define P3 (((unsigned)dst[A] - dst[B]) * 3 + dst[C])
> >   static int do_decode(AVCodecContext *avctx, AVFrame *frame, int decorrelate, int downsample)
> >   {
> > @@ -273,10 +273,10 @@ static int do_decode(AVCodecContext *avctx, AVFrame *frame, int decorrelate, int
> >               case 0:
> >                   break;
> >               case 1:
> > -                dst[n] += dst[A];
> > +                dst[n] += (unsigned)dst[A];
> >                   break;
> >               case 2:
> > -                dst[n] += dst[A] + p;
> > +                dst[n] += (unsigned)dst[A] + p;
> >                   break;
> >               case 3:
> >                   dst[n] += P2;
> > @@ -291,28 +291,28 @@ static int do_decode(AVCodecContext *avctx, AVFrame *frame, int decorrelate, int
> >                   dst[n] += P3 + p;
> >                   break;
> >               case 7:
> > -                dst[n] += (P2 + P3) / 2 + p;
> > +                dst[n] += (int)(P2 + P3) / 2 + (unsigned)p;
> 
> Would 2U work for this? It's shorted and more readable that casts
> everywhere. Same for most cases below.

unsigned and signed division are different
-1 / 2 == 0
-1 / 2U != 0

thx

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

The real ebay dictionary, page 1
"Used only once"    - "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."

[-- 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 2/3] avcodec/osq: avoid several signed integer overflows

[Michael Niedermayer]

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

On Tue, Dec 26, 2023 at 05:37:30PM +0100, Michael Niedermayer wrote:
> Fixes: signed integer overflow: 178459578 + 2009763270 cannot be represented in type 'int'
> Fixes: 62285/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-5013423686287360
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/osq.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)

will apply this and patch 3

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

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin

[-- 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/3] avformat/concatdec: clip outpoint - inpoint overflow in get_best_effort_duration()

[Michael Niedermayer]

An alternative would be to limit all time/duration fields to below 64bit

Fixes: signed integer overflow: -93000000 - 9223372036839000000 cannot be represented in type 'long long'
Fixes: 64546/clusterfuzz-testcase-minimized-ffmpeg_dem_CONCAT_fuzzer-5110813828186112

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

diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c
index 5a7a063ef7d..3da2ac9e705 100644
--- a/libavformat/concatdec.c
+++ b/libavformat/concatdec.c
@@ -323,7 +323,7 @@ static int64_t get_best_effort_duration(ConcatFile *file, AVFormatContext *avf)
     if (file->user_duration != AV_NOPTS_VALUE)
         return file->user_duration;
     if (file->outpoint != AV_NOPTS_VALUE)
-        return file->outpoint - file->file_inpoint;
+        return av_sat_sub64(file->outpoint, file->file_inpoint);
     if (avf->duration > 0)
         return avf->duration - (file->file_inpoint - file->file_start_time);
     if (file->next_dts != AV_NOPTS_VALUE)
-- 
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/3] avcodec/osq: Implement flush()

[Andreas Rheinhardt]

Michael Niedermayer:
> Fixes: out of array access
> Fixes: 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6227491892887552
> Fixes: 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6268561729126400
> Fixes: 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6414805046788096
> Fixes: 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6538151088488448
> Fixes: 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6608131540779008
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/osq.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/libavcodec/osq.c b/libavcodec/osq.c
> index 435f3ea7f69..abe15c97f18 100644
> --- a/libavcodec/osq.c
> +++ b/libavcodec/osq.c
> @@ -61,6 +61,15 @@ typedef struct OSQContext {
>      int pkt_offset;
>  } OSQContext;
>  
> +static void osq_flush(AVCodecContext *avctx)
> +{
> +    OSQContext *s = avctx->priv_data;
> +
> +    s->bitstream_size = 0;
> +    s->pkt_offset = 0;
> +    av_packet_unref(s->pkt);

s->pkt is the same as avctx->internal->in_pkt which is unreferenced
generically upon flush. So this is unnecessary.

> +}
> +
>  static av_cold int osq_close(AVCodecContext *avctx)
>  {
>      OSQContext *s = avctx->priv_data;
> @@ -478,4 +487,5 @@ const FFCodec ff_osq_decoder = {
>                                                          AV_SAMPLE_FMT_S16P,
>                                                          AV_SAMPLE_FMT_S32P,
>                                                          AV_SAMPLE_FMT_NONE },
> +    .flush            = osq_flush,
>  };

_______________________________________________
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/3] avcodec/osq: Implement flush()

[Michael Niedermayer]

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

On Tue, Dec 26, 2023 at 08:09:54PM +0100, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Fixes: out of array access
> > Fixes: 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6227491892887552
> > Fixes: 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6268561729126400
> > Fixes: 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6414805046788096
> > Fixes: 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6538151088488448
> > Fixes: 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_OSQ_fuzzer-6608131540779008
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/osq.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/libavcodec/osq.c b/libavcodec/osq.c
> > index 435f3ea7f69..abe15c97f18 100644
> > --- a/libavcodec/osq.c
> > +++ b/libavcodec/osq.c
> > @@ -61,6 +61,15 @@ typedef struct OSQContext {
> >      int pkt_offset;
> >  } OSQContext;
> >  
> > +static void osq_flush(AVCodecContext *avctx)
> > +{
> > +    OSQContext *s = avctx->priv_data;
> > +
> > +    s->bitstream_size = 0;
> > +    s->pkt_offset = 0;
> > +    av_packet_unref(s->pkt);
> 
> s->pkt is the same as avctx->internal->in_pkt which is unreferenced
> generically upon flush. So this is unnecessary.

will apply without this

thx

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

Nations do behave wisely once they have exhausted all other alternatives. 
-- Abba Eban

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