diff mbox series

[FFmpeg-devel,3/6] avformat/jpegxl_anim_dec: add FF_JPEGXL_CONTAINER_SIGNATURE_LE

Message ID 20230618215021.3044-3-michael@niedermayer.cc
State Accepted
Commit 6a9d13acc26bf02a33b9b8b836bed26d9d43f303
Headers show
Series [FFmpeg-devel,1/6] avformat/jpegxl_anim_dec: Perform operations in a different order | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Michael Niedermayer June 18, 2023, 9:50 p.m. UTC 
Fixes: out of array read
Fixes: 59828/clusterfuzz-testcase-minimized-ffmpeg_dem_JPEGXL_ANIM_fuzzer-5029813220671488

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

Comments

Leo Izen June 18, 2023, 10:57 p.m. UTC | #1 
On 6/18/23 17:50, Michael Niedermayer wrote:
> Fixes: out of array read
> Fixes: 59828/clusterfuzz-testcase-minimized-ffmpeg_dem_JPEGXL_ANIM_fuzzer-5029813220671488
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>   libavformat/jpegxl_anim_dec.c | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/libavformat/jpegxl_anim_dec.c b/libavformat/jpegxl_anim_dec.c
> index c62b596f76..7e4d39385c 100644
> --- a/libavformat/jpegxl_anim_dec.c
> +++ b/libavformat/jpegxl_anim_dec.c
> @@ -108,7 +108,7 @@ static int jpegxl_collect_codestream_header(const uint8_t *input_buffer, int inp
>   
>   static int jpegxl_anim_probe(const AVProbeData *p)
>   {
> -    uint8_t buffer[4096];
> +    uint8_t buffer[4096 + AV_INPUT_BUFFER_PADDING_SIZE];
>       int copied;
>   
>       /* this is a raw codestream */
> @@ -123,7 +123,7 @@ static int jpegxl_anim_probe(const AVProbeData *p)
>       if (AV_RL64(p->buf) != FF_JPEGXL_CONTAINER_SIGNATURE_LE)
>           return 0;
>   
> -    if (jpegxl_collect_codestream_header(p->buf, p->buf_size, buffer, sizeof(buffer), &copied) <= 0 || copied <= 0)
> +    if (jpegxl_collect_codestream_header(p->buf, p->buf_size, buffer, sizeof(buffer) - AV_INPUT_BUFFER_PADDING_SIZE, &copied) <= 0 || copied <= 0)
>           return 0;
>   
>       if (ff_jpegxl_verify_codestream_header(buffer, copied, 0) >= 1)
> @@ -138,7 +138,8 @@ static int jpegxl_anim_read_header(AVFormatContext *s)
>       AVIOContext *pb = s->pb;
>       AVStream *st;
>       int offset = 0;
> -    uint8_t head[256];
> +    uint8_t head[256 + AV_INPUT_BUFFER_PADDING_SIZE];
> +    const int sizeofhead = sizeof(head) - AV_INPUT_BUFFER_PADDING_SIZE;
>       int headsize = 0;
>       int ctrl;
>       AVRational tb;
> @@ -147,7 +148,7 @@ static int jpegxl_anim_read_header(AVFormatContext *s)
>       uint64_t sig16 = avio_rl16(pb);
>       if (sig16 == FF_JPEGXL_CODESTREAM_SIGNATURE_LE) {
>           AV_WL16(head, sig16);
> -        headsize = avio_read(s->pb, head + 2, sizeof(head) - 2);
> +        headsize = avio_read(s->pb, head + 2, sizeofhead - 2);
>           if (headsize < 0)
>               return headsize;
>           headsize += 2;
> @@ -178,10 +179,10 @@ static int jpegxl_anim_read_header(AVFormatContext *s)
>                   if (av_buffer_realloc(&ctx->initial, ctx->initial->size + read) < 0)
>                       return AVERROR(ENOMEM);
>               }
> -            jpegxl_collect_codestream_header(buf, read, head + headsize, sizeof(head) - headsize, &copied);
> +            jpegxl_collect_codestream_header(buf, read, head + headsize, sizeofhead - headsize, &copied);
>               memcpy(ctx->initial->data + (ctx->initial->size - read), buf, read);
>               headsize += copied;
> -            if (headsize >= sizeof(head) || read < sizeof(buf))
> +            if (headsize >= sizeofhead || read < sizeof(buf))
>                   break;
>           }
>       }

What's with the commit message? Seems unrelated to the change.

- Leo Izen
Michael Niedermayer June 19, 2023, 5:01 p.m. UTC | #2 
On Sun, Jun 18, 2023 at 06:57:58PM -0400, Leo Izen wrote:
> On 6/18/23 17:50, Michael Niedermayer wrote:
> > Fixes: out of array read
> > Fixes: 59828/clusterfuzz-testcase-minimized-ffmpeg_dem_JPEGXL_ANIM_fuzzer-5029813220671488
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >   libavformat/jpegxl_anim_dec.c | 13 +++++++------
> >   1 file changed, 7 insertions(+), 6 deletions(-)
> > 
> > diff --git a/libavformat/jpegxl_anim_dec.c b/libavformat/jpegxl_anim_dec.c
> > index c62b596f76..7e4d39385c 100644
> > --- a/libavformat/jpegxl_anim_dec.c
> > +++ b/libavformat/jpegxl_anim_dec.c
> > @@ -108,7 +108,7 @@ static int jpegxl_collect_codestream_header(const uint8_t *input_buffer, int inp
> >   static int jpegxl_anim_probe(const AVProbeData *p)
> >   {
> > -    uint8_t buffer[4096];
> > +    uint8_t buffer[4096 + AV_INPUT_BUFFER_PADDING_SIZE];
> >       int copied;
> >       /* this is a raw codestream */
> > @@ -123,7 +123,7 @@ static int jpegxl_anim_probe(const AVProbeData *p)
> >       if (AV_RL64(p->buf) != FF_JPEGXL_CONTAINER_SIGNATURE_LE)
> >           return 0;
> > -    if (jpegxl_collect_codestream_header(p->buf, p->buf_size, buffer, sizeof(buffer), &copied) <= 0 || copied <= 0)
> > +    if (jpegxl_collect_codestream_header(p->buf, p->buf_size, buffer, sizeof(buffer) - AV_INPUT_BUFFER_PADDING_SIZE, &copied) <= 0 || copied <= 0)
> >           return 0;
> >       if (ff_jpegxl_verify_codestream_header(buffer, copied, 0) >= 1)
> > @@ -138,7 +138,8 @@ static int jpegxl_anim_read_header(AVFormatContext *s)
> >       AVIOContext *pb = s->pb;
> >       AVStream *st;
> >       int offset = 0;
> > -    uint8_t head[256];
> > +    uint8_t head[256 + AV_INPUT_BUFFER_PADDING_SIZE];
> > +    const int sizeofhead = sizeof(head) - AV_INPUT_BUFFER_PADDING_SIZE;
> >       int headsize = 0;
> >       int ctrl;
> >       AVRational tb;
> > @@ -147,7 +148,7 @@ static int jpegxl_anim_read_header(AVFormatContext *s)
> >       uint64_t sig16 = avio_rl16(pb);
> >       if (sig16 == FF_JPEGXL_CODESTREAM_SIGNATURE_LE) {
> >           AV_WL16(head, sig16);
> > -        headsize = avio_read(s->pb, head + 2, sizeof(head) - 2);
> > +        headsize = avio_read(s->pb, head + 2, sizeofhead - 2);
> >           if (headsize < 0)
> >               return headsize;
> >           headsize += 2;
> > @@ -178,10 +179,10 @@ static int jpegxl_anim_read_header(AVFormatContext *s)
> >                   if (av_buffer_realloc(&ctx->initial, ctx->initial->size + read) < 0)
> >                       return AVERROR(ENOMEM);
> >               }
> > -            jpegxl_collect_codestream_header(buf, read, head + headsize, sizeof(head) - headsize, &copied);
> > +            jpegxl_collect_codestream_header(buf, read, head + headsize, sizeofhead - headsize, &copied);
> >               memcpy(ctx->initial->data + (ctx->initial->size - read), buf, read);
> >               headsize += copied;
> > -            if (headsize >= sizeof(head) || read < sizeof(buf))
> > +            if (headsize >= sizeofhead || read < sizeof(buf))
> >                   break;
> >           }
> >       }
> 

> What's with the commit message? Seems unrelated to the change.

Must be some copy and paste mistake



[...]
Leo Izen June 20, 2023, 4:50 p.m. UTC | #3 
On 6/19/23 13:01, Michael Niedermayer wrote:
> On Sun, Jun 18, 2023 at 06:57:58PM -0400, Leo Izen wrote:
>> On 6/18/23 17:50, Michael Niedermayer wrote:
>>> Fixes: out of array read
>>> Fixes: 59828/clusterfuzz-testcase-minimized-ffmpeg_dem_JPEGXL_ANIM_fuzzer-5029813220671488
>>>
>>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>>> ---
>>>    libavformat/jpegxl_anim_dec.c | 13 +++++++------
>>>    1 file changed, 7 insertions(+), 6 deletions(-)
> 
>> What's with the commit message? Seems unrelated to the change.
> 
> Must be some copy and paste mistake

Pushed with a different commit message as 6a9d13acc26b.

- Leo Izen
diff mbox series

Patch

diff --git a/libavformat/jpegxl_anim_dec.c b/libavformat/jpegxl_anim_dec.c
index c62b596f76..7e4d39385c 100644
--- a/libavformat/jpegxl_anim_dec.c
+++ b/libavformat/jpegxl_anim_dec.c
@@ -108,7 +108,7 @@  static int jpegxl_collect_codestream_header(const uint8_t *input_buffer, int inp
 
 static int jpegxl_anim_probe(const AVProbeData *p)
 {
-    uint8_t buffer[4096];
+    uint8_t buffer[4096 + AV_INPUT_BUFFER_PADDING_SIZE];
     int copied;
 
     /* this is a raw codestream */
@@ -123,7 +123,7 @@  static int jpegxl_anim_probe(const AVProbeData *p)
     if (AV_RL64(p->buf) != FF_JPEGXL_CONTAINER_SIGNATURE_LE)
         return 0;
 
-    if (jpegxl_collect_codestream_header(p->buf, p->buf_size, buffer, sizeof(buffer), &copied) <= 0 || copied <= 0)
+    if (jpegxl_collect_codestream_header(p->buf, p->buf_size, buffer, sizeof(buffer) - AV_INPUT_BUFFER_PADDING_SIZE, &copied) <= 0 || copied <= 0)
         return 0;
 
     if (ff_jpegxl_verify_codestream_header(buffer, copied, 0) >= 1)
@@ -138,7 +138,8 @@  static int jpegxl_anim_read_header(AVFormatContext *s)
     AVIOContext *pb = s->pb;
     AVStream *st;
     int offset = 0;
-    uint8_t head[256];
+    uint8_t head[256 + AV_INPUT_BUFFER_PADDING_SIZE];
+    const int sizeofhead = sizeof(head) - AV_INPUT_BUFFER_PADDING_SIZE;
     int headsize = 0;
     int ctrl;
     AVRational tb;
@@ -147,7 +148,7 @@  static int jpegxl_anim_read_header(AVFormatContext *s)
     uint64_t sig16 = avio_rl16(pb);
     if (sig16 == FF_JPEGXL_CODESTREAM_SIGNATURE_LE) {
         AV_WL16(head, sig16);
-        headsize = avio_read(s->pb, head + 2, sizeof(head) - 2);
+        headsize = avio_read(s->pb, head + 2, sizeofhead - 2);
         if (headsize < 0)
             return headsize;
         headsize += 2;
@@ -178,10 +179,10 @@  static int jpegxl_anim_read_header(AVFormatContext *s)
                 if (av_buffer_realloc(&ctx->initial, ctx->initial->size + read) < 0)
                     return AVERROR(ENOMEM);
             }
-            jpegxl_collect_codestream_header(buf, read, head + headsize, sizeof(head) - headsize, &copied);
+            jpegxl_collect_codestream_header(buf, read, head + headsize, sizeofhead - headsize, &copied);
             memcpy(ctx->initial->data + (ctx->initial->size - read), buf, read);
             headsize += copied;
-            if (headsize >= sizeof(head) || read < sizeof(buf))
+            if (headsize >= sizeofhead || read < sizeof(buf))
                 break;
         }
     }