[FFmpeg-devel] [PATCH 1/2] avcodec/flac_parser: Assert that we do not overrun the link_penalty array

[FFmpeg-devel] [PATCH 1/2] avcodec/flac_parser: Assert that we do not overrun the link_penalty array

[Michael Niedermayer]

Fixes: CID1454676 Out-of-bounds read

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

diff --git a/libavcodec/flac_parser.c b/libavcodec/flac_parser.c
index 47904d515a6..d9c47801f83 100644
--- a/libavcodec/flac_parser.c
+++ b/libavcodec/flac_parser.c
@@ -518,6 +518,8 @@ static int check_header_mismatch(FLACParseContext  *fpc,
         for (i = 0; i < FLAC_MAX_SEQUENTIAL_HEADERS && curr != child; i++)
             curr = curr->next;
 
+        av_assert0(i < FLAC_MAX_SEQUENTIAL_HEADERS);
+
         if (header->link_penalty[i] < FLAC_HEADER_CRC_FAIL_PENALTY ||
             header->link_penalty[i] == FLAC_HEADER_NOT_PENALIZED_YET) {
             FLACHeaderMarker *start, *end;
-- 
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/2] avcodec/fmvc: remove dead assignment

[Michael Niedermayer]

Fixes: CID1529220 Unused value

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

diff --git a/libavcodec/fmvc.c b/libavcodec/fmvc.c
index 30f7aaf6bc3..b51b18a9eec 100644
--- a/libavcodec/fmvc.c
+++ b/libavcodec/fmvc.c
@@ -101,7 +101,6 @@ static int decode_type2(GetByteContext *gb, PutByteContext *pb)
                             continue;
                         }
                     }
-                    repeat = 0;
                 }
                 repeat = 1;
             }
-- 
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/2] avcodec/fmvc: remove dead assignment

[Michael Niedermayer]

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

On Sun, May 05, 2024 at 01:52:00AM +0200, Michael Niedermayer wrote:
> Fixes: CID1529220 Unused value
> 
> Sponsored-by: Sovereign Tech Fund
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/fmvc.c | 1 -
>  1 file changed, 1 deletion(-)

will apply

[...]
-- 
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 1/2] avcodec/flac_parser: Assert that we do not overrun the link_penalty array

[Rémi Denis-Courmont]

Le 5 mai 2024 02:51:59 GMT+03:00, Michael Niedermayer <michael@niedermayer.cc> a écrit :
>Fixes: CID1454676 Out-of-bounds read

It's a stretch to call this "fixing". It just asserts that the situation doesn't happen, in other words, that it is a false positive from the static analyser.

The code change looks OK, but the commit description seems misleading.

>
>Sponsored-by: Sovereign Tech Fund
>Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>---
> libavcodec/flac_parser.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/libavcodec/flac_parser.c b/libavcodec/flac_parser.c
>index 47904d515a6..d9c47801f83 100644
>--- a/libavcodec/flac_parser.c
>+++ b/libavcodec/flac_parser.c
>@@ -518,6 +518,8 @@ static int check_header_mismatch(FLACParseContext  *fpc,
>         for (i = 0; i < FLAC_MAX_SEQUENTIAL_HEADERS && curr != child; i++)
>             curr = curr->next;
> 
>+        av_assert0(i < FLAC_MAX_SEQUENTIAL_HEADERS);
>+
>         if (header->link_penalty[i] < FLAC_HEADER_CRC_FAIL_PENALTY ||
>             header->link_penalty[i] == FLAC_HEADER_NOT_PENALIZED_YET) {
>             FLACHeaderMarker *start, *end;
_______________________________________________
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/2] avcodec/flac_parser: Assert that we do not overrun the link_penalty array

[Michael Niedermayer]

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

Hi

On Mon, May 13, 2024 at 09:07:50AM +0300, Rémi Denis-Courmont wrote:
> 
> 
> Le 5 mai 2024 02:51:59 GMT+03:00, Michael Niedermayer <michael@niedermayer.cc> a écrit :
> >Fixes: CID1454676 Out-of-bounds read
> 
> It's a stretch to call this "fixing". It just asserts that the situation doesn't happen,

yes


> in other words, that it is a false positive from the static analyser.

thanks for reviewing


> 
> The code change looks OK, but the commit description seems misleading.

will apply with "Helps: CID1454676 Out-of-bounds read" instead if "Fixing: ..."

thx

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

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- 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 1/2] avcodec/flac_parser: Assert that we do not overrun the link_penalty array

[Andreas Rheinhardt]

Michael Niedermayer:
> Fixes: CID1454676 Out-of-bounds read
> 
> Sponsored-by: Sovereign Tech Fund
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/flac_parser.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/libavcodec/flac_parser.c b/libavcodec/flac_parser.c
> index 47904d515a6..d9c47801f83 100644
> --- a/libavcodec/flac_parser.c
> +++ b/libavcodec/flac_parser.c
> @@ -518,6 +518,8 @@ static int check_header_mismatch(FLACParseContext  *fpc,
>          for (i = 0; i < FLAC_MAX_SEQUENTIAL_HEADERS && curr != child; i++)
>              curr = curr->next;
>  
> +        av_assert0(i < FLAC_MAX_SEQUENTIAL_HEADERS);
> +
>          if (header->link_penalty[i] < FLAC_HEADER_CRC_FAIL_PENALTY ||
>              header->link_penalty[i] == FLAC_HEADER_NOT_PENALIZED_YET) {
>              FLACHeaderMarker *start, *end;

If this is only supposed to mark an issue as invalid for the sanitizer,
why are you adding an av_assert0 instead of av_assert1 here (and in
other patches)?

- Andreas

_______________________________________________
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/2] avcodec/flac_parser: Assert that we do not overrun the link_penalty array

[Michael Niedermayer]

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

On Mon, May 13, 2024 at 10:45:16PM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Fixes: CID1454676 Out-of-bounds read
> > 
> > Sponsored-by: Sovereign Tech Fund
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/flac_parser.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/libavcodec/flac_parser.c b/libavcodec/flac_parser.c
> > index 47904d515a6..d9c47801f83 100644
> > --- a/libavcodec/flac_parser.c
> > +++ b/libavcodec/flac_parser.c
> > @@ -518,6 +518,8 @@ static int check_header_mismatch(FLACParseContext  *fpc,
> >          for (i = 0; i < FLAC_MAX_SEQUENTIAL_HEADERS && curr != child; i++)
> >              curr = curr->next;
> >  
> > +        av_assert0(i < FLAC_MAX_SEQUENTIAL_HEADERS);
> > +
> >          if (header->link_penalty[i] < FLAC_HEADER_CRC_FAIL_PENALTY ||
> >              header->link_penalty[i] == FLAC_HEADER_NOT_PENALIZED_YET) {
> >              FLACHeaderMarker *start, *end;
> 
> If this is only supposed to mark an issue as invalid for the sanitizer,
> why are you adding an av_assert0 instead of av_assert1 here

The flac parser code is complex and confusing me a bit

If i would write av_assert1() then i would be saying that iam 100% sure this
is true and i certainly do not feel that confident. Thats why its av_assert0
and also why i have neither marked this in coverity a false positive nor a bug.
I was hoping posting this to the mailing list would result in either someone
confirming it to be correct or telling me that iam an idiot and that this is
wrong. And it seemed remi agreed that the change is correct so i intended to
push it but iam happy to wait if you or someone else wants to take a look

thx



> (and in
> other patches)?
> 
> - Andreas
> 
> _______________________________________________
> 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".
> 

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

During times of universal deceit, telling the truth becomes a
revolutionary act. -- George Orwell

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