[FFmpeg-devel] [PATCH 1/4] avcodec/mscc: move frame allocates to later

[FFmpeg-devel] [PATCH 1/4] avcodec/mscc: move frame allocates to later

[Michael Niedermayer]

Fixes: Timeout
Fixes: 66964/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SRGC_fuzzer-5413170363564032

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/mscc.c | 43 +++++++++++++++++++++----------------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/libavcodec/mscc.c b/libavcodec/mscc.c
index d1d23e6751..1e9fd35f03 100644
--- a/libavcodec/mscc.c
+++ b/libavcodec/mscc.c
@@ -142,28 +142,6 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *frame,
     if (avpkt->size < 3)
         return buf_size;
 
-    if ((ret = ff_get_buffer(avctx, frame, 0)) < 0)
-        return ret;
-
-    if (avctx->pix_fmt == AV_PIX_FMT_PAL8) {
-        size_t size;
-        const uint8_t *pal = av_packet_get_side_data(avpkt, AV_PKT_DATA_PALETTE, &size);
-
-        if (pal && size == AVPALETTE_SIZE) {
-#if FF_API_PALETTE_HAS_CHANGED
-FF_DISABLE_DEPRECATION_WARNINGS
-            frame->palette_has_changed = 1;
-FF_ENABLE_DEPRECATION_WARNINGS
-#endif
-            for (j = 0; j < 256; j++)
-                s->pal[j] = 0xFF000000 | AV_RL32(pal + j * 4);
-        } else if (pal) {
-            av_log(avctx, AV_LOG_ERROR,
-                   "Palette size %"SIZE_SPECIFIER" is wrong\n", size);
-        }
-        memcpy(frame->data[1], s->pal, AVPALETTE_SIZE);
-    }
-
     ret = inflateReset(zstream);
     if (ret != Z_OK) {
         av_log(avctx, AV_LOG_ERROR, "Inflate reset error: %d\n", ret);
@@ -191,6 +169,27 @@ inflate_error:
         av_log(avctx, AV_LOG_ERROR, "Inflate error: %d\n", ret);
         return AVERROR_UNKNOWN;
     }
+    if ((ret = ff_get_buffer(avctx, frame, 0)) < 0)
+        return ret;
+
+    if (avctx->pix_fmt == AV_PIX_FMT_PAL8) {
+        size_t size;
+        const uint8_t *pal = av_packet_get_side_data(avpkt, AV_PKT_DATA_PALETTE, &size);
+
+        if (pal && size == AVPALETTE_SIZE) {
+#if FF_API_PALETTE_HAS_CHANGED
+FF_DISABLE_DEPRECATION_WARNINGS
+            frame->palette_has_changed = 1;
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
+            for (j = 0; j < 256; j++)
+                s->pal[j] = 0xFF000000 | AV_RL32(pal + j * 4);
+        } else if (pal) {
+            av_log(avctx, AV_LOG_ERROR,
+                   "Palette size %"SIZE_SPECIFIER" is wrong\n", size);
+        }
+        memcpy(frame->data[1], s->pal, AVPALETTE_SIZE);
+    }
 
     bytestream2_init(&gb, s->decomp_buf, zstream->total_out);
     bytestream2_init_writer(&pb, s->uncomp_buf, s->uncomp_size);
-- 
2.17.1

_______________________________________________
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/4] avformat/iamf_reader: return REDO on failure to read

[Michael Niedermayer]

Fixes: null pointer derference
Fixes: 67007/clusterfuzz-testcase-minimized-ffmpeg_dem_IAMF_fuzzer-6522819204677632

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_reader.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libavformat/iamf_reader.c b/libavformat/iamf_reader.c
index 42d20f1ae6..a06aa98cdb 100644
--- a/libavformat/iamf_reader.c
+++ b/libavformat/iamf_reader.c
@@ -26,6 +26,7 @@
 #include "libavcodec/packet.h"
 #include "avformat.h"
 #include "avio_internal.h"
+#include "demux.h"
 #include "iamf.h"
 #include "iamf_parse.h"
 #include "iamf_reader.h"
@@ -322,7 +323,7 @@ int ff_iamf_read_packet(AVFormatContext *s, IAMFDemuxContext *c,
             break;
     }
 
-    return read;
+    return FFERROR_REDO;
 }
 
 void ff_iamf_read_deinit(IAMFDemuxContext *c)
-- 
2.17.1

_______________________________________________
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/4] avformat/iamf_reader: return REDO on failure to read

[James Almer]

On 3/20/2024 10:15 PM, Michael Niedermayer wrote:
> Fixes: null pointer derference
> Fixes: 67007/clusterfuzz-testcase-minimized-ffmpeg_dem_IAMF_fuzzer-6522819204677632
> 
> 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_reader.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/iamf_reader.c b/libavformat/iamf_reader.c
> index 42d20f1ae6..a06aa98cdb 100644
> --- a/libavformat/iamf_reader.c
> +++ b/libavformat/iamf_reader.c
> @@ -26,6 +26,7 @@
>   #include "libavcodec/packet.h"
>   #include "avformat.h"
>   #include "avio_internal.h"
> +#include "demux.h"
>   #include "iamf.h"
>   #include "iamf_parse.h"
>   #include "iamf_reader.h"
> @@ -322,7 +323,7 @@ int ff_iamf_read_packet(AVFormatContext *s, IAMFDemuxContext *c,
>               break;
>       }
>   
> -    return read;
> +    return FFERROR_REDO;

Where is the null pointer dereference happening? I don't particularly 
like this approach because ff_iamf_read_packet() is also called by the 
mov demuxer.

>   }
>   
>   void ff_iamf_read_deinit(IAMFDemuxContext *c)

Does the following also help?

> diff --git a/libavformat/iamf_reader.c b/libavformat/iamf_reader.c
> index 42d20f1ae6..4e79691a03 100644
> --- a/libavformat/iamf_reader.c
> +++ b/libavformat/iamf_reader.c
> @@ -311,8 +311,7 @@ int ff_iamf_read_packet(AVFormatContext *s, IAMFDemuxContext *c,
>          } else {
>              int64_t offset = avio_skip(pb, obu_size);
>              if (offset < 0) {
> -                ret = offset;
> -                break;
> +                return offset;
>              }
>          }
>          max_size -= len;

Setting ret there and breaking the loop was wrong, as the scope of ret 
doesn't reach outside loop.
_______________________________________________
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/4] avformat/iamf_reader: return REDO on failure to read

[Michael Niedermayer]

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

On Wed, Mar 20, 2024 at 11:22:11PM -0300, James Almer wrote:
> On 3/20/2024 10:15 PM, Michael Niedermayer wrote:
> > Fixes: null pointer derference
> > Fixes: 67007/clusterfuzz-testcase-minimized-ffmpeg_dem_IAMF_fuzzer-6522819204677632
> > 
> > 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_reader.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavformat/iamf_reader.c b/libavformat/iamf_reader.c
> > index 42d20f1ae6..a06aa98cdb 100644
> > --- a/libavformat/iamf_reader.c
> > +++ b/libavformat/iamf_reader.c
> > @@ -26,6 +26,7 @@
> >   #include "libavcodec/packet.h"
> >   #include "avformat.h"
> >   #include "avio_internal.h"
> > +#include "demux.h"
> >   #include "iamf.h"
> >   #include "iamf_parse.h"
> >   #include "iamf_reader.h"
> > @@ -322,7 +323,7 @@ int ff_iamf_read_packet(AVFormatContext *s, IAMFDemuxContext *c,
> >               break;
> >       }
> > -    return read;
> > +    return FFERROR_REDO;
> 
> Where is the null pointer dereference happening? I don't particularly like
> this approach because ff_iamf_read_packet() is also called by the mov
> demuxer.

==8458==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x000000703a8b bp 0x7ffc691161f0 sp 0x7ffc69116000 T0)
==8458==The signal is caused by a READ memory access.
==8458==Hint: address points to the zero page.
    #0 0x703a8a in ff_read_packet ffmpeg/libavformat/demux.c:671:15
    #1 0x7074cc in read_frame_internal ffmpeg/libavformat/demux.c:1346:15
    #2 0x704a07 in av_read_frame ffmpeg/libavformat/demux.c:1550:17
    #3 0x4c844f in LLVMFuzzerTestOneInput ffmpeg/tools/target_dem_fuzzer.c:211:15


	SCARINESS: 10 (null-deref)
    #0 0x60b7cc in ff_read_packet /src/ffmpeg/libavformat/demux.c:683:15
    #1 0x60cbcb in read_frame_internal /src/ffmpeg/libavformat/demux.c:1358:15
    #2 0x60c59b in av_read_frame /src/ffmpeg/libavformat/demux.c:1569:17
    #3 0x57808a in LLVMFuzzerTestOneInput /src/ffmpeg/tools/target_dem_fuzzer.c:211:15
    #4 0x449183 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15
    #5 0x4348e2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
    #6 0x43a18c in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:860:9
    #7 0x4636c2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    #8 0x7854642cc082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/libc-start.c:308:16
    #9 0x42aaad in _start

Either way the iamf demuxer returns no packet but the caller believes it
returns a packet. WHen that gets used things crash


> 
> >   }
> >   void ff_iamf_read_deinit(IAMFDemuxContext *c)
> 
> Does the following also help?
> 
> > diff --git a/libavformat/iamf_reader.c b/libavformat/iamf_reader.c
> > index 42d20f1ae6..4e79691a03 100644
> > --- a/libavformat/iamf_reader.c
> > +++ b/libavformat/iamf_reader.c
> > @@ -311,8 +311,7 @@ int ff_iamf_read_packet(AVFormatContext *s, IAMFDemuxContext *c,
> >          } else {
> >              int64_t offset = avio_skip(pb, obu_size);
> >              if (offset < 0) {
> > -                ret = offset;
> > -                break;
> > +                return offset;
> >              }
> >          }
> >          max_size -= len;

yes, this seems to fix it too and is better

thx

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

Any man who breaks a law that conscience tells him is unjust and willingly 
accepts the penalty by staying in jail in order to arouse the conscience of 
the community on the injustice of the law is at that moment expressing the 
very highest respect for law. - Martin Luther King Jr

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

[FFmpeg-devel] [PATCH 3/4] avcodec/cbs_h266_syntax_template: Check tile_y

[Michael Niedermayer]

Fixes: out of array access
Fixes: 67021/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-4883576579489792

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/cbs_h266_syntax_template.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavcodec/cbs_h266_syntax_template.c b/libavcodec/cbs_h266_syntax_template.c
index 49fb12ba77..0aae9fdfd5 100644
--- a/libavcodec/cbs_h266_syntax_template.c
+++ b/libavcodec/cbs_h266_syntax_template.c
@@ -2072,6 +2072,8 @@ static int FUNC(pps) (CodedBitstreamContext *ctx, RWContext *rw,
 
                 tile_x = tile_idx % current->num_tile_columns;
                 tile_y = tile_idx / current->num_tile_columns;
+                if (tile_y >= current->num_tile_rows)
+                    return AVERROR_INVALIDDATA;
 
                 ctu_x = 0, ctu_y = 0;
                 for (j = 0; j < tile_x; j++) {
-- 
2.17.1

_______________________________________________
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 3/4] avcodec/cbs_h266_syntax_template: Check tile_y

[Nuo Mi]

On Thu, Mar 21, 2024 at 9:15 AM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> Fixes: out of array access
> Fixes:
> 67021/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-4883576579489792
>
> Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by
> <https://github.com/google/oss-fuzz/tree/master/projects/ffmpegSigned-off-by>:
> Michael Niedermayer <michael@niedermayer.cc>
>

Pushed this one.
Thank you, Michael.

> ---
>  libavcodec/cbs_h266_syntax_template.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/libavcodec/cbs_h266_syntax_template.c
> b/libavcodec/cbs_h266_syntax_template.c
> index 49fb12ba77..0aae9fdfd5 100644
> --- a/libavcodec/cbs_h266_syntax_template.c
> +++ b/libavcodec/cbs_h266_syntax_template.c
> @@ -2072,6 +2072,8 @@ static int FUNC(pps) (CodedBitstreamContext *ctx,
> RWContext *rw,
>
>                  tile_x = tile_idx % current->num_tile_columns;
>                  tile_y = tile_idx / current->num_tile_columns;
> +                if (tile_y >= current->num_tile_rows)
> +                    return AVERROR_INVALIDDATA;
>
>                  ctu_x = 0, ctu_y = 0;
>                  for (j = 0; j < tile_x; j++) {
> --
> 2.17.1
>
> _______________________________________________
> 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 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 4/4] avformat/iamf: Check language_label

[Michael Niedermayer]

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)
+        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);
 }
-- 
2.17.1

_______________________________________________
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 4/4] avformat/iamf: Check language_label

[James Almer]

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;
>      }

_______________________________________________
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 4/4] avformat/iamf: Check language_label

[Michael Niedermayer]

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

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

[Michael Niedermayer]

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

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

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