On Thu, Aug 01, 2024 at 05:11:18PM +0200, Lynne via ffmpeg-devel wrote:
> On 31/07/2024 21:54, Michael Niedermayer wrote:
> > Fixes: out of array access
> > Fixes: 70734/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AAC_LATM_fuzzer-4741427068731392
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >   libavcodec/aac/aacdec_usac.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/aac/aacdec_usac.c b/libavcodec/aac/aacdec_usac.c
> > index 82db65eb0d0..2938e693874 100644
> > --- a/libavcodec/aac/aacdec_usac.c
> > +++ b/libavcodec/aac/aacdec_usac.c
> > @@ -918,8 +918,10 @@ static int decode_usac_stereo_info(AACDecContext *ac, AACUSACConfig *usac,
> >           }
> >           ret = setup_sce(ac, sce1, usac);
> > -        if (ret < 0)
> > +        if (ret < 0) {
> > +            ics2->max_sfb = 0;
> >               return ret;
> > +        }
> >           ret = setup_sce(ac, sce2, usac);
> >           if (ret < 0)
> 
> Err, the one and only place where setup_sce can return an error is also
> where ics->max_sfb = 0; is cleaned up. It doesn't make sense that this patch
> would do anything at all.

there are 2 single channel elements
when the first fails, it automatically cleans the firsts max_sfb but as is before
this patch it leaves the 2nd SCE max_sfb unchanged to whatever unchecked value
was put in it.
It would get checked and cleared in the next setup_sce() call but that is
never called if the first fails

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is what and why we do it that matters, not just one of them.