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

[Michael Niedermayer]

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 at 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 at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> 

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

During times of universal deceit, telling the truth becomes a
revolutionary act. -- George Orwell
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20240514/de4527c7/attachment.sig>