diff mbox series

[FFmpeg-devel] libavcodec/png_parser.c: fix a use_of_uninitialized_value in target_dec_fuzzer.

Message ID 20200604184033.66758-1-tfoucu@gmail.com
State Accepted
Headers show
Series [FFmpeg-devel] libavcodec/png_parser.c: fix a use_of_uninitialized_value in target_dec_fuzzer.
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Thierry Foucu June 4, 2020, 6:40 p.m. UTC
target_dec_fuzzer is checking for the avpkt.data pointer but if the
png parser cannot combine the frame, the poutbuf is not set and so, the
avpkt.data is not initialized.
---
 libavcodec/png_parser.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

James Almer June 4, 2020, 6:52 p.m. UTC | #1
On 6/4/2020 3:40 PM, Thierry Foucu wrote:
> target_dec_fuzzer is checking for the avpkt.data pointer but if the
> png parser cannot combine the frame, the poutbuf is not set and so, the
> avpkt.data is not initialized.
> ---
>  libavcodec/png_parser.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/png_parser.c b/libavcodec/png_parser.c
> index 74f2964118..d1b2926154 100644
> --- a/libavcodec/png_parser.c
> +++ b/libavcodec/png_parser.c
> @@ -99,8 +99,11 @@ static int png_parse(AVCodecParserContext *s, AVCodecContext *avctx,
>      }
>  
>  flush:
> -    if (ff_combine_frame(&ppc->pc, next, &buf, &buf_size) < 0)
> +    if (ff_combine_frame(&ppc->pc, next, &buf, &buf_size) < 0) {
> +        *poutbuf = NULL;
> +        *poutbuf_size = 0;

This is already set to 0 first thing in png_parse(), so i'd say we
should also set *poutbuf to NULL there instead.

And I see the BMP parser has the same issue, so it needs the same fix.

>          return buf_size;
> +    }
>  
>      ppc->chunk_pos = ppc->pc.frame_start_found = 0;
>  
>
Thierry Foucu June 4, 2020, 7:59 p.m. UTC | #2
On Thu, Jun 4, 2020 at 11:52 AM James Almer <jamrial@gmail.com> wrote:

> On 6/4/2020 3:40 PM, Thierry Foucu wrote:
> > target_dec_fuzzer is checking for the avpkt.data pointer but if the
> > png parser cannot combine the frame, the poutbuf is not set and so, the
> > avpkt.data is not initialized.
> > ---
> >  libavcodec/png_parser.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/png_parser.c b/libavcodec/png_parser.c
> > index 74f2964118..d1b2926154 100644
> > --- a/libavcodec/png_parser.c
> > +++ b/libavcodec/png_parser.c
> > @@ -99,8 +99,11 @@ static int png_parse(AVCodecParserContext *s,
> AVCodecContext *avctx,
> >      }
> >
> >  flush:
> > -    if (ff_combine_frame(&ppc->pc, next, &buf, &buf_size) < 0)
> > +    if (ff_combine_frame(&ppc->pc, next, &buf, &buf_size) < 0) {
> > +        *poutbuf = NULL;
> > +        *poutbuf_size = 0;
>
> This is already set to 0 first thing in png_parse(), so i'd say we
> should also set *poutbuf to NULL there instead.
>
> Good catch.. I will modify the patch.


> And I see the BMP parser has the same issue, so it needs the same fix.
>
> I will send a patch for BMP and also for MLP. Both have the same problem.


> >          return buf_size;
> > +    }
> >
> >      ppc->chunk_pos = ppc->pc.frame_start_found = 0;
> >
> >
>
> _______________________________________________
> 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".
diff mbox series

Patch

diff --git a/libavcodec/png_parser.c b/libavcodec/png_parser.c
index 74f2964118..d1b2926154 100644
--- a/libavcodec/png_parser.c
+++ b/libavcodec/png_parser.c
@@ -99,8 +99,11 @@  static int png_parse(AVCodecParserContext *s, AVCodecContext *avctx,
     }
 
 flush:
-    if (ff_combine_frame(&ppc->pc, next, &buf, &buf_size) < 0)
+    if (ff_combine_frame(&ppc->pc, next, &buf, &buf_size) < 0) {
+        *poutbuf = NULL;
+        *poutbuf_size = 0;
         return buf_size;
+    }
 
     ppc->chunk_pos = ppc->pc.frame_start_found = 0;