On Thu, Mar 21, 2024 at 04:55:46AM +0100, Michael Niedermayer wrote:
> On Wed, Mar 20, 2024 at 11:17:09PM -0300, James Almer wrote:
> > On 3/20/2024 10:15 PM, Michael Niedermayer wrote:
> > > Fixes: null pointer dereference
> > > Fixes: 67023/clusterfuzz-testcase-minimized-ffmpeg_dem_IAMF_fuzzer-6011025237278720
> > > 
> > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > ---
> > >   libavformat/iamf.c | 7 ++++---
> > >   1 file changed, 4 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/libavformat/iamf.c b/libavformat/iamf.c
> > > index 5de70dc082..f2c22ce3aa 100644
> > > --- a/libavformat/iamf.c
> > > +++ b/libavformat/iamf.c
> > > @@ -89,9 +89,10 @@ void ff_iamf_free_mix_presentation(IAMFMixPresentation **pmix_presentation)
> > >       if (!mix_presentation)
> > >           return;
> > > -    for (int i = 0; i < mix_presentation->count_label; i++)
> > > -        av_free(mix_presentation->language_label[i]);
> > > -    av_free(mix_presentation->language_label);
> > > +    if (mix_presentation->language_label)
> > 
> > If count_label is not 0, then language_label should be allocated.
> > 
> > > +        for (int i = 0; i < mix_presentation->count_label; i++)
> > > +            av_free(mix_presentation->language_label[i]);
> > > +    av_freep(&mix_presentation->language_label);
> > >       av_iamf_mix_presentation_free(&mix_presentation->mix);
> > >       av_freep(pmix_presentation);
> > >   }
> > 
> > Can you test the following?
> > 
> > > diff --git a/libavformat/iamf_parse.c b/libavformat/iamf_parse.c
> > > index cb49cf0a57..e29c2c6b6c 100644
> > > --- a/libavformat/iamf_parse.c
> > > +++ b/libavformat/iamf_parse.c
> > > @@ -822,6 +822,7 @@ static int mix_presentation_obu(void *s, IAMFContext *c, AVIOContext *pb, int le
> > >      mix_presentation->language_label = av_calloc(mix_presentation->count_label,
> > >                                                   sizeof(*mix_presentation->language_label));
> > >      if (!mix_presentation->language_label) {
> > > +        mix_presentation->count_label = 0;
> > >          ret = AVERROR(ENOMEM);
> > >          goto fail;
> > >      }
> 
> that works too, i think pointers should be set to NULL on deallocation though

i will apply this alternative

txh

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch