[FFmpeg-devel] [PATCH 1/5] avcodec/lpc: copy levenson coeffs only when they have been computed

[FFmpeg-devel] [PATCH 1/5] avcodec/lpc: copy levenson coeffs only when they have been computed

[Michael Niedermayer]

Fixes: CID1473514 Uninitialized scalar variable

Sponsored-by: Sovereign Tech Fund
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/lpc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libavcodec/lpc.c b/libavcodec/lpc.c
index 8305cc0596a..981dacce8a5 100644
--- a/libavcodec/lpc.c
+++ b/libavcodec/lpc.c
@@ -282,8 +282,9 @@ int ff_lpc_calc_coefs(LPCContext *s,
         double av_uninit(weight);
         memset(var, 0, FFALIGN(MAX_LPC_ORDER+1,4)*sizeof(*var));
 
-        for(j=0; j<max_order; j++)
-            m[0].coeff[max_order-1][j] = -lpc[max_order-1][j];
+        if (lpc_passes > 1)
+            for(j=0; j<max_order; j++)
+                m[0].coeff[max_order-1][j] = -lpc[max_order-1][j];
 
         for(; pass<lpc_passes; pass++){
             avpriv_init_lls(&m[pass&1], max_order);
-- 
2.43.2

_______________________________________________
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/mpeg12dec: Use 64bit in bit computation

[Michael Niedermayer]

I dont think this can actually overflow but 64bit seems reasonable to use

Fixes: CID1521983 Unintentional integer overflow

Sponsored-by: Sovereign Tech Fund
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/mpeg12dec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 21a214ef5b7..e257889d034 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -2734,7 +2734,7 @@ static int ipu_decode_frame(AVCodecContext *avctx, AVFrame *frame,
     int ret;
 
     // Check for minimal intra MB size (considering mb header, luma & chroma dc VLC, ac EOB VLC)
-    if (avpkt->size*8LL < (avctx->width+15)/16 * ((avctx->height+15)/16) * (2 + 3*4 + 2*2 + 2*6))
+    if (avpkt->size*8LL < (avctx->width+15)/16 * ((avctx->height+15)/16) * (2LL + 3*4 + 2*2 + 2*6))
         return AVERROR_INVALIDDATA;
 
     ret = ff_get_buffer(avctx, frame, 0);
-- 
2.43.2

_______________________________________________
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/mpeg12dec: Use 64bit in bit computation

[Michael Niedermayer]

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

On Sun, May 12, 2024 at 02:03:46AM +0200, Michael Niedermayer wrote:
> I dont think this can actually overflow but 64bit seems reasonable to use
> 
> Fixes: CID1521983 Unintentional integer overflow
> 
> Sponsored-by: Sovereign Tech Fund
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/mpeg12dec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

will apply patch 2 - 5


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

You can kill me, but you cannot change the truth.

[-- 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/5] avcodec/mpeg4videodec: assert impossible wrap points

[Michael Niedermayer]

Helps: CID1473517 Uninitialized scalar variable
Helps: CID1473497 Uninitialized scalar variable

Sponsored-by: Sovereign Tech Fund
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/mpeg4videodec.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c
index 6a7a37e8171..df1e22207db 100644
--- a/libavcodec/mpeg4videodec.c
+++ b/libavcodec/mpeg4videodec.c
@@ -597,6 +597,8 @@ static int mpeg4_decode_sprite_trajectory(Mpeg4DecContext *ctx, GetBitContext *g
         ctx->sprite_shift[0]  = alpha + beta + rho - min_ab;
         ctx->sprite_shift[1]  = alpha + beta + rho - min_ab + 2;
         break;
+    default:
+        av_assert0(0);
     }
     /* try to simplify the situation */
     if (sprite_delta[0][0] == a << ctx->sprite_shift[0] &&
-- 
2.43.2

_______________________________________________
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/mpegvideo_enc: Fix potential overflow in RD

[Michael Niedermayer]

Fixes: CID1500285 Unintentional integer overflow

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

diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index b601a1a9e40..73a9082265b 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -1433,7 +1433,7 @@ static int estimate_best_b_count(MpegEncContext *s)
                 goto fail;
             }
 
-            rd += (out_size * lambda2) >> (FF_LAMBDA_SHIFT - 3);
+            rd += (out_size * (uint64_t)lambda2) >> (FF_LAMBDA_SHIFT - 3);
         }
 
         /* get the delayed frames */
@@ -1442,7 +1442,7 @@ static int estimate_best_b_count(MpegEncContext *s)
             ret = out_size;
             goto fail;
         }
-        rd += (out_size * lambda2) >> (FF_LAMBDA_SHIFT - 3);
+        rd += (out_size * (uint64_t)lambda2) >> (FF_LAMBDA_SHIFT - 3);
 
         rd += c->error[0] + c->error[1] + c->error[2];
 
-- 
2.43.2

_______________________________________________
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/mscc & mwsc: Check loop counts before use

[Michael Niedermayer]

This could cause timeouts

Fixes: CID1439568 Untrusted loop bound

Sponsored-by: Sovereign Tech Fund
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/mscc.c |  6 ++++++
 libavcodec/mwsc.c | 11 +++++++++++
 2 files changed, 17 insertions(+)

diff --git a/libavcodec/mscc.c b/libavcodec/mscc.c
index 39bfad0b989..0c11fa08a24 100644
--- a/libavcodec/mscc.c
+++ b/libavcodec/mscc.c
@@ -54,6 +54,9 @@ static int rle_uncompress(AVCodecContext *avctx, GetByteContext *gb, PutByteCont
         unsigned run = bytestream2_get_byte(gb);
 
         if (run) {
+            if (bytestream2_get_bytes_left_p(pb) < run * s->bpp)
+                return AVERROR_INVALIDDATA;
+
             switch (avctx->bits_per_coded_sample) {
             case 8:
                 fill = bytestream2_get_byte(gb);
@@ -102,6 +105,9 @@ static int rle_uncompress(AVCodecContext *avctx, GetByteContext *gb, PutByteCont
 
                 bytestream2_seek_p(pb, y * avctx->width * s->bpp + x * s->bpp, SEEK_SET);
             } else {
+                if (bytestream2_get_bytes_left_p(pb) < copy * s->bpp)
+                    return AVERROR_INVALIDDATA;
+
                 for (j = 0; j < copy; j++) {
                     switch (avctx->bits_per_coded_sample) {
                     case 8:
diff --git a/libavcodec/mwsc.c b/libavcodec/mwsc.c
index 06a151a72af..0d4ee9791ad 100644
--- a/libavcodec/mwsc.c
+++ b/libavcodec/mwsc.c
@@ -51,6 +51,10 @@ static int rle_uncompress(GetByteContext *gb, PutByteContext *pb, GetByteContext
 
         if (run == 0) {
             run = bytestream2_get_le32(gb);
+
+            if (bytestream2_tell_p(pb) + width - w < run)
+                return AVERROR_INVALIDDATA;
+
             for (int j = 0; j < run; j++, w++) {
                 if (w == width) {
                     w = 0;
@@ -62,6 +66,10 @@ static int rle_uncompress(GetByteContext *gb, PutByteContext *pb, GetByteContext
             int pos = bytestream2_tell_p(pb);
 
             bytestream2_seek(gbp, pos, SEEK_SET);
+
+            if (pos + width - w < fill)
+                return AVERROR_INVALIDDATA;
+
             for (int j = 0; j < fill; j++, w++) {
                 if (w == width) {
                     w = 0;
@@ -73,6 +81,9 @@ static int rle_uncompress(GetByteContext *gb, PutByteContext *pb, GetByteContext
 
             intra = 0;
         } else {
+            if (bytestream2_tell_p(pb) + width - w < run)
+                return AVERROR_INVALIDDATA;
+
             for (int j = 0; j < run; j++, w++) {
                 if (w == width) {
                     w = 0;
-- 
2.43.2

_______________________________________________
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/5] avcodec/lpc: copy levenson coeffs only when they have been computed

[Lynne via ffmpeg-devel]

On 12/05/2024 02:03, Michael Niedermayer wrote:
> Fixes: CID1473514 Uninitialized scalar variable
> 
> Sponsored-by: Sovereign Tech Fund
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>   libavcodec/lpc.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/lpc.c b/libavcodec/lpc.c
> index 8305cc0596a..981dacce8a5 100644
> --- a/libavcodec/lpc.c
> +++ b/libavcodec/lpc.c
> @@ -282,8 +282,9 @@ int ff_lpc_calc_coefs(LPCContext *s,
>           double av_uninit(weight);
>           memset(var, 0, FFALIGN(MAX_LPC_ORDER+1,4)*sizeof(*var));
>   
> -        for(j=0; j<max_order; j++)
> -            m[0].coeff[max_order-1][j] = -lpc[max_order-1][j];
> +        if (lpc_passes > 1)
> +            for(j=0; j<max_order; j++)
> +                m[0].coeff[max_order-1][j] = -lpc[max_order-1][j];
>   
>           for(; pass<lpc_passes; pass++){
>               avpriv_init_lls(&m[pass&1], max_order);

max_order is a function argument, I don't think that's the right place 
to fix this.
_______________________________________________
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/5] avcodec/lpc: copy levenson coeffs only when they have been computed

[Michael Niedermayer]

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

On Sun, May 12, 2024 at 02:13:06AM +0200, Lynne via ffmpeg-devel wrote:
> On 12/05/2024 02:03, Michael Niedermayer wrote:
> > Fixes: CID1473514 Uninitialized scalar variable
> > 
> > Sponsored-by: Sovereign Tech Fund
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >   libavcodec/lpc.c | 5 +++--
> >   1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavcodec/lpc.c b/libavcodec/lpc.c
> > index 8305cc0596a..981dacce8a5 100644
> > --- a/libavcodec/lpc.c
> > +++ b/libavcodec/lpc.c
> > @@ -282,8 +282,9 @@ int ff_lpc_calc_coefs(LPCContext *s,
> >           double av_uninit(weight);
> >           memset(var, 0, FFALIGN(MAX_LPC_ORDER+1,4)*sizeof(*var));
> > -        for(j=0; j<max_order; j++)
> > -            m[0].coeff[max_order-1][j] = -lpc[max_order-1][j];
> > +        if (lpc_passes > 1)
> > +            for(j=0; j<max_order; j++)
> > +                m[0].coeff[max_order-1][j] = -lpc[max_order-1][j];
> >           for(; pass<lpc_passes; pass++){
> >               avpriv_init_lls(&m[pass&1], max_order);
> 
> max_order is a function argument, I don't think that's the right place to
> fix this.

max_orders is fine

what the problem is, is that CHOLESKY with lpc_passes = 1
skips the first pass LEVINSON but this line copies the output
from LEVINSON so it copies Uninitialized data.
a few lines later thats cleared with avpriv_init_lls()
but that access to uninitialized data i think is undefined behavior

if my analysis is not wrong then i think my fix is correct

thx

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

Re: [FFmpeg-devel] [PATCH 1/5] avcodec/lpc: copy levenson coeffs only when they have been computed

[Lynne via ffmpeg-devel]

On 12/05/2024 02:18, Michael Niedermayer wrote:
> On Sun, May 12, 2024 at 02:13:06AM +0200, Lynne via ffmpeg-devel wrote:
>> On 12/05/2024 02:03, Michael Niedermayer wrote:
>>> Fixes: CID1473514 Uninitialized scalar variable
>>>
>>> Sponsored-by: Sovereign Tech Fund
>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>>> ---
>>>    libavcodec/lpc.c | 5 +++--
>>>    1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavcodec/lpc.c b/libavcodec/lpc.c
>>> index 8305cc0596a..981dacce8a5 100644
>>> --- a/libavcodec/lpc.c
>>> +++ b/libavcodec/lpc.c
>>> @@ -282,8 +282,9 @@ int ff_lpc_calc_coefs(LPCContext *s,
>>>            double av_uninit(weight);
>>>            memset(var, 0, FFALIGN(MAX_LPC_ORDER+1,4)*sizeof(*var));
>>> -        for(j=0; j<max_order; j++)
>>> -            m[0].coeff[max_order-1][j] = -lpc[max_order-1][j];
>>> +        if (lpc_passes > 1)
>>> +            for(j=0; j<max_order; j++)
>>> +                m[0].coeff[max_order-1][j] = -lpc[max_order-1][j];
>>>            for(; pass<lpc_passes; pass++){
>>>                avpriv_init_lls(&m[pass&1], max_order);
>>
>> max_order is a function argument, I don't think that's the right place to
>> fix this.
> 
> max_orders is fine
> 
> what the problem is, is that CHOLESKY with lpc_passes = 1
> skips the first pass LEVINSON but this line copies the output
> from LEVINSON so it copies Uninitialized data.
> a few lines later thats cleared with avpriv_init_lls()
> but that access to uninitialized data i think is undefined behavior
> 
> if my analysis is not wrong then i think my fix is correct
> 
> thx
> 
> [...]
> 
> 
> _______________________________________________
> 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".

Ah, I see. Could you put a small comment above, like:
/* Avoids initializing with an unused value when max_order == 1 */?
Other than that looks fine.
_______________________________________________
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/5] avcodec/lpc: copy levenson coeffs only when they have been computed

[Michael Niedermayer]

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

On Sun, May 12, 2024 at 02:29:57AM +0200, Lynne via ffmpeg-devel wrote:
> On 12/05/2024 02:18, Michael Niedermayer wrote:
> > On Sun, May 12, 2024 at 02:13:06AM +0200, Lynne via ffmpeg-devel wrote:
> > > On 12/05/2024 02:03, Michael Niedermayer wrote:
> > > > Fixes: CID1473514 Uninitialized scalar variable
> > > > 
> > > > Sponsored-by: Sovereign Tech Fund
> > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > > ---
> > > >    libavcodec/lpc.c | 5 +++--
> > > >    1 file changed, 3 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/libavcodec/lpc.c b/libavcodec/lpc.c
> > > > index 8305cc0596a..981dacce8a5 100644
> > > > --- a/libavcodec/lpc.c
> > > > +++ b/libavcodec/lpc.c
> > > > @@ -282,8 +282,9 @@ int ff_lpc_calc_coefs(LPCContext *s,
> > > >            double av_uninit(weight);
> > > >            memset(var, 0, FFALIGN(MAX_LPC_ORDER+1,4)*sizeof(*var));
> > > > -        for(j=0; j<max_order; j++)
> > > > -            m[0].coeff[max_order-1][j] = -lpc[max_order-1][j];
> > > > +        if (lpc_passes > 1)
> > > > +            for(j=0; j<max_order; j++)
> > > > +                m[0].coeff[max_order-1][j] = -lpc[max_order-1][j];
> > > >            for(; pass<lpc_passes; pass++){
> > > >                avpriv_init_lls(&m[pass&1], max_order);
> > > 
> > > max_order is a function argument, I don't think that's the right place to
> > > fix this.
> > 
> > max_orders is fine
> > 
> > what the problem is, is that CHOLESKY with lpc_passes = 1
> > skips the first pass LEVINSON but this line copies the output
> > from LEVINSON so it copies Uninitialized data.
> > a few lines later thats cleared with avpriv_init_lls()
> > but that access to uninitialized data i think is undefined behavior
> > 
> > if my analysis is not wrong then i think my fix is correct
> > 
> > thx
> > 
> > [...]
> > 
> > 
> > _______________________________________________
> > 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".
> 
> Ah, I see. Could you put a small comment above, like:
> /* Avoids initializing with an unused value when max_order == 1 */?
> Other than that looks fine.

ok, will apply with such note

thx

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

There will always be a question for which you do not know the correct answer.

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