[FFmpeg-devel] [PATCH 1/4] avcodec/jpegxl_parser: ensure input padding is zeroed

[FFmpeg-devel] [PATCH 1/4] avcodec/jpegxl_parser: ensure input padding is zeroed

[Kacper Michajłow]

Fixes use of uninitialized value, reported by MSAN.

Found by OSS-Fuzz.

Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
---
 libavcodec/jpegxl_parser.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavcodec/jpegxl_parser.c b/libavcodec/jpegxl_parser.c
index 8c45e1a1b7..f833f844c4 100644
--- a/libavcodec/jpegxl_parser.c
+++ b/libavcodec/jpegxl_parser.c
@@ -1419,6 +1419,7 @@ static int try_parse(AVCodecParserContext *s, AVCodecContext *avctx, JXLParseCon
         }
         cs_buffer = ctx->cs_buffer;
         cs_buflen = FFMIN(sizeof(ctx->cs_buffer) - AV_INPUT_BUFFER_PADDING_SIZE, ctx->copied);
+        memset(ctx->cs_buffer + cs_buflen, 0, AV_INPUT_BUFFER_PADDING_SIZE);
     } else {
         cs_buffer = buf;
         cs_buflen = buf_size;
-- 
2.43.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".

[FFmpeg-devel] [PATCH 2/4] avcodec/parser: ensure input padding is zeroed

[Kacper Michajłow]

Fixes use of uninitialized value, reported by MSAN.

Found by OSS-Fuzz.

Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
---
 libavcodec/parser.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavcodec/parser.c b/libavcodec/parser.c
index af17ee9c15..426cc314fb 100644
--- a/libavcodec/parser.c
+++ b/libavcodec/parser.c
@@ -236,6 +236,7 @@ int ff_combine_frame(ParseContext *pc, int next,
         }
         pc->buffer = new_buffer;
         memcpy(&pc->buffer[pc->index], *buf, *buf_size);
+        memset(&pc->buffer[pc->index + *buf_size], 0, AV_INPUT_BUFFER_PADDING_SIZE);
         pc->index += *buf_size;
         return -1;
     }
-- 
2.43.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".

[FFmpeg-devel] [PATCH 3/4] avformat/img2dec: ensure input padding is zeroed

[Kacper Michajłow]

Fixes use of uninitialized value, reported by MSAN. Specifically in
jpegxl parser.

Found by OSS-Fuzz.

Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
---
 libavformat/img2dec.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c
index ba52353074..c667d8574c 100644
--- a/libavformat/img2dec.c
+++ b/libavformat/img2dec.c
@@ -549,6 +549,8 @@ int ff_img_read_packet(AVFormatContext *s1, AVPacket *pkt)
         }
     }
 
+    memset(pkt->data + pkt->size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
+
     if (ret[0] <= 0 || ret[1] < 0 || ret[2] < 0) {
         if (ret[0] < 0) {
             res = ret[0];
-- 
2.43.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".

[FFmpeg-devel] [PATCH 4/4] avformat/jpegxl_anim_dec: ensure input padding is zeroed

[Kacper Michajłow]

Fixes use of uninitialized value, reported by MSAN.

Found by OSS-Fuzz.

Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
---
 libavformat/jpegxl_anim_dec.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavformat/jpegxl_anim_dec.c b/libavformat/jpegxl_anim_dec.c
index ac95d3b961..2338a2e8c0 100644
--- a/libavformat/jpegxl_anim_dec.c
+++ b/libavformat/jpegxl_anim_dec.c
@@ -124,6 +124,8 @@ static int jpegxl_anim_read_header(AVFormatContext *s)
         }
     }
 
+    memset(head + headsize, 0, AV_INPUT_BUFFER_PADDING_SIZE);
+
     /* offset in bits of the animation header */
     ret = ff_jpegxl_parse_codestream_header(head, headsize, &meta, 0);
     if (ret < 0 || meta.animation_offset <= 0)
-- 
2.43.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".

Re: [FFmpeg-devel] [PATCH 1/4] avcodec/jpegxl_parser: ensure input padding is zeroed

[Andreas Rheinhardt]

Kacper Michajłow:
> Fixes use of uninitialized value, reported by MSAN.
> 
> Found by OSS-Fuzz.
> 
> Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
> ---
>  libavcodec/jpegxl_parser.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavcodec/jpegxl_parser.c b/libavcodec/jpegxl_parser.c
> index 8c45e1a1b7..f833f844c4 100644
> --- a/libavcodec/jpegxl_parser.c
> +++ b/libavcodec/jpegxl_parser.c
> @@ -1419,6 +1419,7 @@ static int try_parse(AVCodecParserContext *s, AVCodecContext *avctx, JXLParseCon
>          }
>          cs_buffer = ctx->cs_buffer;
>          cs_buflen = FFMIN(sizeof(ctx->cs_buffer) - AV_INPUT_BUFFER_PADDING_SIZE, ctx->copied);
> +        memset(ctx->cs_buffer + cs_buflen, 0, AV_INPUT_BUFFER_PADDING_SIZE);
>      } else {
>          cs_buffer = buf;
>          cs_buflen = buf_size;

This is very strange: The buffer here is part of the parser's private
context and this is zeroed initially. So it should never contain
uninitialized values.

- Andreas

_______________________________________________
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] avformat/img2dec: ensure input padding is zeroed

[Paul B Mahol]

On Thu, Jun 27, 2024 at 3:57 AM Kacper Michajłow <kasper93@gmail.com> wrote:

> Fixes use of uninitialized value, reported by MSAN. Specifically in
> jpegxl parser.
>
> Found by OSS-Fuzz.
>
> Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
> ---
>  libavformat/img2dec.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c
> index ba52353074..c667d8574c 100644
> --- a/libavformat/img2dec.c
> +++ b/libavformat/img2dec.c
> @@ -549,6 +549,8 @@ int ff_img_read_packet(AVFormatContext *s1, AVPacket
> *pkt)
>          }
>      }
>
> +    memset(pkt->data + pkt->size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
> +
>      if (ret[0] <= 0 || ret[1] < 0 || ret[2] < 0) {
>          if (ret[0] < 0) {
>              res = ret[0];
> --
> 2.43.0
>
>
Isn't this done generically already?
Otherwise this fix is just fixing one single case of numerous others not
covered.


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

Re: [FFmpeg-devel] [PATCH 3/4] avformat/img2dec: ensure input padding is zeroed

[Andreas Rheinhardt]

Paul B Mahol:
> On Thu, Jun 27, 2024 at 3:57 AM Kacper Michajłow <kasper93@gmail.com> wrote:
> 
>> Fixes use of uninitialized value, reported by MSAN. Specifically in
>> jpegxl parser.
>>
>> Found by OSS-Fuzz.
>>
>> Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
>> ---
>>  libavformat/img2dec.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c
>> index ba52353074..c667d8574c 100644
>> --- a/libavformat/img2dec.c
>> +++ b/libavformat/img2dec.c
>> @@ -549,6 +549,8 @@ int ff_img_read_packet(AVFormatContext *s1, AVPacket
>> *pkt)
>>          }
>>      }
>>
>> +    memset(pkt->data + pkt->size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
>> +
>>      if (ret[0] <= 0 || ret[1] < 0 || ret[2] < 0) {
>>          if (ret[0] < 0) {
>>              res = ret[0];
>> --
>> 2.43.0
>>
>>
> Isn't this done generically already?
> Otherwise this fix is just fixing one single case of numerous others not
> covered.
> 

av_new_packet() is zeroing the padding, but in case that less is read
than intended the bytes after the payload bytes are uninitialized.

- Andreas

_______________________________________________
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 1/4] avcodec/jpegxl_parser: ensure input padding is zeroed

[Kacper Michajlow]

On Thu, 27 Jun 2024 at 02:55, Andreas Rheinhardt
<andreas.rheinhardt@outlook.com> wrote:
>
> Kacper Michajłow:
> > Fixes use of uninitialized value, reported by MSAN.
> >
> > Found by OSS-Fuzz.
> >
> > Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
> > ---
> >  libavcodec/jpegxl_parser.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/libavcodec/jpegxl_parser.c b/libavcodec/jpegxl_parser.c
> > index 8c45e1a1b7..f833f844c4 100644
> > --- a/libavcodec/jpegxl_parser.c
> > +++ b/libavcodec/jpegxl_parser.c
> > @@ -1419,6 +1419,7 @@ static int try_parse(AVCodecParserContext *s, AVCodecContext *avctx, JXLParseCon
> >          }
> >          cs_buffer = ctx->cs_buffer;
> >          cs_buflen = FFMIN(sizeof(ctx->cs_buffer) - AV_INPUT_BUFFER_PADDING_SIZE, ctx->copied);
> > +        memset(ctx->cs_buffer + cs_buflen, 0, AV_INPUT_BUFFER_PADDING_SIZE);
> >      } else {
> >          cs_buffer = buf;
> >          cs_buflen = buf_size;
>
> This is very strange: The buffer here is part of the parser's private
> context and this is zeroed initially. So it should never contain
> uninitialized values.

You are right, but reading this code I had the feeling that it would
have the same issue, we read only ctx->copied amount of bytes. We can
skip this one if this is certain that this buffer will be clear before
reuse always.
_______________________________________________
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 1/4] avcodec/jpegxl_parser: ensure input padding is zeroed

[Leo Izen]

On 6/26/24 8:40 PM, Kacper Michajłow wrote:
> Fixes use of uninitialized value, reported by MSAN.
> 
> Found by OSS-Fuzz.
> 
> Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
> ---
>   libavcodec/jpegxl_parser.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/libavcodec/jpegxl_parser.c b/libavcodec/jpegxl_parser.c
> index 8c45e1a1b7..f833f844c4 100644
> --- a/libavcodec/jpegxl_parser.c
> +++ b/libavcodec/jpegxl_parser.c
> @@ -1419,6 +1419,7 @@ static int try_parse(AVCodecParserContext *s, AVCodecContext *avctx, JXLParseCon
>           }
>           cs_buffer = ctx->cs_buffer;
>           cs_buflen = FFMIN(sizeof(ctx->cs_buffer) - AV_INPUT_BUFFER_PADDING_SIZE, ctx->copied);
> +        memset(ctx->cs_buffer + cs_buflen, 0, AV_INPUT_BUFFER_PADDING_SIZE);
>       } else {
>           cs_buffer = buf;
>           cs_buflen = buf_size;

This one looks fine. Per the other comments on the other jxl patch, I 
think that one is unnecessary, but this one is fine.

I have no comment on the generic patches.

- Leo Izen (Traneptora)
_______________________________________________
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".