Re: [FFmpeg-devel] [PATCH 4/4] avformat/iamf: Check language_label

[Michael Niedermayer <michael@niedermayer.cc>]

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

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

thx

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

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.

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