[FFmpeg-devel] [PATCH 1/5] avcodec/mv30: Allocate frame later

[FFmpeg-devel] [PATCH 1/5] avcodec/mv30: Allocate frame later

[Michael Niedermayer]

This way we can check the input before allocation in the next commit

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/mv30.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/libavcodec/mv30.c b/libavcodec/mv30.c
index c2d0547053..0b19534b00 100644
--- a/libavcodec/mv30.c
+++ b/libavcodec/mv30.c
@@ -414,6 +414,9 @@ static int decode_intra(AVCodecContext *avctx, GetBitContext *gb, AVFrame *frame
 
     skip_bits_long(gb, s->mode_size * 8);
 
+    if ((ret = ff_get_buffer(avctx, frame, AV_GET_BUFFER_FLAG_REF)) < 0)
+        return ret;
+
     linesize[0] = frame->linesize[0];
     linesize[1] = frame->linesize[0];
     linesize[2] = frame->linesize[0];
@@ -473,6 +476,9 @@ static int decode_inter(AVCodecContext *avctx, GetBitContext *gb,
     int ret, cnt = 0;
     int flags = 0;
 
+    if ((ret = ff_get_buffer(avctx, frame, AV_GET_BUFFER_FLAG_REF)) < 0)
+        return ret;
+
     in_linesize[0] = prev->linesize[0];
     in_linesize[1] = prev->linesize[0];
     in_linesize[2] = prev->linesize[0];
@@ -610,9 +616,6 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *frame,
     if ((ret = init_get_bits8(gb, avpkt->data, avpkt->size)) < 0)
         return ret;
 
-    if ((ret = ff_get_buffer(avctx, frame, AV_GET_BUFFER_FLAG_REF)) < 0)
-        return ret;
-
     s->intra_quant = get_bits(gb, 8);
     s->inter_quant = s->intra_quant + get_sbits(gb, 8);
     s->is_inter = get_bits_le(gb, 16);
-- 
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/5] avcodec/mv30: Check the input length before allocation

[Michael Niedermayer]

Fixes: Timeout
Fixes: 60867/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MV30_fuzzer-6381933108527104

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

diff --git a/libavcodec/mv30.c b/libavcodec/mv30.c
index 0b19534b00..a5d272762b 100644
--- a/libavcodec/mv30.c
+++ b/libavcodec/mv30.c
@@ -411,6 +411,8 @@ static int decode_intra(AVCodecContext *avctx, GetBitContext *gb, AVFrame *frame
     mgb = *gb;
     if (get_bits_left(gb) < s->mode_size * 8)
         return AVERROR_INVALIDDATA;
+    if (get_bits_left(gb) < 2 * 6 * (avctx->height / 16) * (avctx->width / 16))
+        return AVERROR_INVALIDDATA;
 
     skip_bits_long(gb, s->mode_size * 8);
 
@@ -476,6 +478,9 @@ static int decode_inter(AVCodecContext *avctx, GetBitContext *gb,
     int ret, cnt = 0;
     int flags = 0;
 
+    if (get_bits_left(gb) < (mask_size + s->mode_size) * 8)
+        return AVERROR_INVALIDDATA;
+
     if ((ret = ff_get_buffer(avctx, frame, AV_GET_BUFFER_FLAG_REF)) < 0)
         return ret;
 
-- 
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/5] avcodec/mv30: Check the input length before allocation

[Paul B Mahol]

NAK
_______________________________________________
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/5] avcodec/mv30: Check the input length before allocation

[Michael Niedermayer]

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

On Mon, Aug 07, 2023 at 10:22:25AM +0200, Paul B Mahol wrote:
> NAK

will apply unless you provide technical comments

thx

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

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato

[-- 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/5] avcodec/mv30: Check the input length before allocation

[Paul B Mahol]

On 9/22/23, Michael Niedermayer <michael@niedermayer.cc> wrote:
> On Mon, Aug 07, 2023 at 10:22:25AM +0200, Paul B Mahol wrote:
>> NAK
>
> will apply unless you provide technical comments

NAK, never provided proof that this hack does not break decoding.
This is not really security fix.

>
> thx
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Good people do not need laws to tell them to act responsibly, while bad
> people will find a way around the laws. -- Plato
>
_______________________________________________
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/5] avcodec/mv30: Check the input length before allocation

[Michael Niedermayer]

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

On Fri, Sep 22, 2023 at 09:31:39PM +0200, Paul B Mahol wrote:
> On 9/22/23, Michael Niedermayer <michael@niedermayer.cc> wrote:
> > On Mon, Aug 07, 2023 at 10:22:25AM +0200, Paul B Mahol wrote:
> >> NAK
> >
> > will apply unless you provide technical comments
> 
> NAK, never provided proof that this hack does not break decoding.

i think i did in
"[FFmpeg-devel] [PATCH v2] avcodec/mv30: Check the input length before allocation"

it seems i replied to the older of these fixes by mistake, but really
they do the same

[...]

thx

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Some people wanted to paint the bikeshed green, some blue and some pink.
People argued and fought, when they finally agreed, only rust was left.

[-- 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/5] avcodec/mv30: Check the input length before allocation

[Paul B Mahol]

On 9/22/23, Michael Niedermayer <michael@niedermayer.cc> wrote:
> On Fri, Sep 22, 2023 at 09:31:39PM +0200, Paul B Mahol wrote:
>> On 9/22/23, Michael Niedermayer <michael@niedermayer.cc> wrote:
>> > On Mon, Aug 07, 2023 at 10:22:25AM +0200, Paul B Mahol wrote:
>> >> NAK
>> >
>> > will apply unless you provide technical comments
>>
>> NAK, never provided proof that this hack does not break decoding.
>
> i think i did in
> "[FFmpeg-devel] [PATCH v2] avcodec/mv30: Check the input length before
> allocation"
>
> it seems i replied to the older of these fixes by mistake, but really
> they do the same

If this one appears to need to be reverted because it breaks decoding
than you should resign from your current role.

>
> [...]
>
> thx
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Some people wanted to paint the bikeshed green, some blue and some pink.
> People argued and fought, when they finally agreed, only rust was left.
>
_______________________________________________
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/5] avcodec/xvididct: Fix integer overflow in idct_row()

[Michael Niedermayer]

Fixes: signed integer overflow: -1403461578 + -843974775 cannot be represented in type 'int'
Fixes: 60868/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MPEG1VIDEO_fuzzer-4599793035378688

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

diff --git a/libavcodec/xvididct.c b/libavcodec/xvididct.c
index f338901ab2..43ea927437 100644
--- a/libavcodec/xvididct.c
+++ b/libavcodec/xvididct.c
@@ -69,24 +69,24 @@ static int idct_row(short *in, const int *const tab, int rnd)
     if (!(right | in[4])) {
         const int k = c4 * in[0] + rnd;
         if (left) {
-            const int a0 = k + c2 * in[2];
-            const int a1 = k + c6 * in[2];
-            const int a2 = k - c6 * in[2];
-            const int a3 = k - c2 * in[2];
+            const unsigned a0 = k + c2 * in[2];
+            const unsigned a1 = k + c6 * in[2];
+            const unsigned a2 = k - c6 * in[2];
+            const unsigned a3 = k - c2 * in[2];
 
             const int b0 = c1 * in[1] + c3 * in[3];
             const int b1 = c3 * in[1] - c7 * in[3];
             const int b2 = c5 * in[1] - c1 * in[3];
             const int b3 = c7 * in[1] - c5 * in[3];
 
-            in[0] = (a0 + b0) >> ROW_SHIFT;
-            in[1] = (a1 + b1) >> ROW_SHIFT;
-            in[2] = (a2 + b2) >> ROW_SHIFT;
-            in[3] = (a3 + b3) >> ROW_SHIFT;
-            in[4] = (a3 - b3) >> ROW_SHIFT;
-            in[5] = (a2 - b2) >> ROW_SHIFT;
-            in[6] = (a1 - b1) >> ROW_SHIFT;
-            in[7] = (a0 - b0) >> ROW_SHIFT;
+            in[0] = (int)(a0 + b0) >> ROW_SHIFT;
+            in[1] = (int)(a1 + b1) >> ROW_SHIFT;
+            in[2] = (int)(a2 + b2) >> ROW_SHIFT;
+            in[3] = (int)(a3 + b3) >> ROW_SHIFT;
+            in[4] = (int)(a3 - b3) >> ROW_SHIFT;
+            in[5] = (int)(a2 - b2) >> ROW_SHIFT;
+            in[6] = (int)(a1 - b1) >> ROW_SHIFT;
+            in[7] = (int)(a0 - b0) >> ROW_SHIFT;
         } else {
             const int a0 = k >> ROW_SHIFT;
             if (a0) {
-- 
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 3/5] avcodec/xvididct: Fix integer overflow in idct_row()

[Michael Niedermayer]

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

On Mon, Aug 07, 2023 at 02:49:47AM +0200, Michael Niedermayer wrote:
> Fixes: signed integer overflow: -1403461578 + -843974775 cannot be represented in type 'int'
> Fixes: 60868/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MPEG1VIDEO_fuzzer-4599793035378688
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/xvididct.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)

Will apply patches 3-5

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

It is dangerous to be right in matters on which the established authorities
are wrong. -- 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".

[FFmpeg-devel] [PATCH 4/5] avcodec/wavarc: Check shift

[Michael Niedermayer]

Fixes: shift exponent 1285 is too large for 32-bit type 'int'
Fixes: 60870/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WAVARC_fuzzer-5332050340347904

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

diff --git a/libavcodec/wavarc.c b/libavcodec/wavarc.c
index 8e4c7d17dc..0dc5849679 100644
--- a/libavcodec/wavarc.c
+++ b/libavcodec/wavarc.c
@@ -218,6 +218,10 @@ static int decode_1dif(AVCodecContext *avctx,
             continue;
         case 6:
             s->shift = get_urice(gb, 2);
+            if ((unsigned)s->shift > 31) {
+                s->shift = 0;
+                return AVERROR_INVALIDDATA;
+            }
             continue;
         case 5:
             if (avctx->sample_fmt == AV_SAMPLE_FMT_U8P) {
@@ -314,6 +318,10 @@ static int decode_2slp(AVCodecContext *avctx,
             continue;
         case 7:
             s->shift = get_urice(gb, 2);
+            if ((unsigned)s->shift > 31) {
+                s->shift = 0;
+                return AVERROR_INVALIDDATA;
+            }
             continue;
         case 6:
             if (avctx->sample_fmt == AV_SAMPLE_FMT_U8P) {
@@ -586,6 +594,10 @@ static int decode_5elp(AVCodecContext *avctx,
             continue;
         case 10:
             s->shift = get_urice(gb, 2);
+            if ((unsigned)s->shift > 31) {
+                s->shift = 0;
+                return AVERROR_INVALIDDATA;
+            }
             continue;
         case 9:
             if (avctx->sample_fmt == AV_SAMPLE_FMT_U8P) {
-- 
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 5/5] avcodec/wavarc: Check that nb_samples is not negative

[Michael Niedermayer]

It is currently probably not possible for it to be negative as
the needed 2Mb input buf size is not achievable. But it is more
robust to check for it too.
If it would become negative than code like
s->samples[0][n] = s->samples[0][s->nb_samples + n];
would crash

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

diff --git a/libavcodec/wavarc.c b/libavcodec/wavarc.c
index 0dc5849679..4bdd548d5f 100644
--- a/libavcodec/wavarc.c
+++ b/libavcodec/wavarc.c
@@ -311,7 +311,7 @@ static int decode_2slp(AVCodecContext *avctx,
             return AVERROR_EOF;
         case 8:
             s->nb_samples = get_urice(gb, 8);
-            if (s->nb_samples > 570) {
+            if (s->nb_samples > 570U) {
                 s->nb_samples = 570;
                 return AVERROR_INVALIDDATA;
             }
@@ -587,7 +587,7 @@ static int decode_5elp(AVCodecContext *avctx,
             return AVERROR_EOF;
         case 11:
             s->nb_samples = get_urice(gb, 8);
-            if (s->nb_samples > 570) {
+            if (s->nb_samples > 570U) {
                 s->nb_samples = 570;
                 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".