[FFmpeg-devel] [PATCH] avcodec/jpegxl_parse{, r}: fix integer overflow for some malformed files

[FFmpeg-devel] [PATCH] avcodec/jpegxl_parse{, r}: fix integer overflow for some malformed files

[Leo Izen]

If there's a very large ISOBMFF box that needs to be skipped, it can
cause an overflow for ctx->skip. There's already a safeguard to return
quickly if ctx->skip > bufsize, so changing ctx->skip to int64_t will
allow this to happen even if ctx->skip would overflow a signed int.

Several other members are also changed to int64_t to avoid this problem
in other possible scenarios.

Signed-off-by: Leo Izen <leo.izen@gmail.com>
Reported-by: Kacper Michajlow <kasper93@gmail.com>
Fixes: clusterfuzz-testcase-minimized-fuzzer_loadfile-6085331937460224
---
 libavcodec/jpegxl_parse.c  |  5 +++--
 libavcodec/jpegxl_parser.c | 16 ++++++++--------
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/libavcodec/jpegxl_parse.c b/libavcodec/jpegxl_parse.c
index 7cfdd3e7d5..022eed322d 100644
--- a/libavcodec/jpegxl_parse.c
+++ b/libavcodec/jpegxl_parse.c
@@ -450,7 +450,8 @@ int ff_jpegxl_collect_codestream_header(const uint8_t *input_buffer, int input_l
                                         uint8_t *buffer, int buflen, int *copied)
 {
     GetByteContext gb;
-    int pos = 0, last_box = 0;
+    int64_t pos = 0;
+    int last_box = 0;
     bytestream2_init(&gb, input_buffer, input_len);
 
     while (1) {
@@ -516,5 +517,5 @@ int ff_jpegxl_collect_codestream_header(const uint8_t *input_buffer, int input_l
             break;
     }
 
-    return pos;
+    return FFMIN(pos, INT_MAX);
 }
diff --git a/libavcodec/jpegxl_parser.c b/libavcodec/jpegxl_parser.c
index a888e9ae6e..68404229a5 100644
--- a/libavcodec/jpegxl_parser.c
+++ b/libavcodec/jpegxl_parser.c
@@ -155,12 +155,12 @@ typedef struct JXLParseContext {
 
     /* using ISOBMFF-based container */
     int container;
-    int skip;
+    int64_t skip;
     int copied;
-    int collected_size;
-    int codestream_length;
+    int64_t collected_size;
+    int64_t codestream_length;
     int skipped_icc;
-    int next;
+    int64_t next;
 
     uint8_t cs_buffer[4096 + AV_INPUT_BUFFER_PADDING_SIZE];
 } JXLParseContext;
@@ -1396,7 +1396,7 @@ static int skip_boxes(JXLParseContext *ctx, const uint8_t *buf, int buf_size)
     return 0;
 }
 
-static int try_parse(AVCodecParserContext *s, AVCodecContext *avctx, JXLParseContext *ctx,
+static int64_t try_parse(AVCodecParserContext *s, AVCodecContext *avctx, JXLParseContext *ctx,
                      const uint8_t *buf, int buf_size)
 {
     int ret, cs_buflen, header_skip;
@@ -1489,10 +1489,10 @@ static int jpegxl_parse(AVCodecParserContext *s, AVCodecContext *avctx,
     }
 
     if ((!ctx->container || !ctx->codestream_length) && !ctx->next) {
-        ret = try_parse(s, avctx, ctx, pbuf, pindex);
-        if (ret < 0)
+        int64_t ret64 = try_parse(s, avctx, ctx, pbuf, pindex);
+        if (ret64 < 0)
             goto flush;
-        ctx->next = ret;
+        ctx->next = ret64;
         if (ctx->container)
             ctx->skip += ctx->next;
     }
-- 
2.48.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] avcodec/jpegxl_parse{, r}: fix integer overflow for some malformed files

[Kacper Michajlow]

On Wed, 29 Jan 2025 at 20:58, Leo Izen <leo.izen@gmail.com> wrote:
>
> If there's a very large ISOBMFF box that needs to be skipped, it can
> cause an overflow for ctx->skip. There's already a safeguard to return
> quickly if ctx->skip > bufsize, so changing ctx->skip to int64_t will
> allow this to happen even if ctx->skip would overflow a signed int.
>
> Several other members are also changed to int64_t to avoid this problem
> in other possible scenarios.
>
> Signed-off-by: Leo Izen <leo.izen@gmail.com>
> Reported-by: Kacper Michajlow <kasper93@gmail.com>
> Fixes: clusterfuzz-testcase-minimized-fuzzer_loadfile-6085331937460224
> ---
>  libavcodec/jpegxl_parse.c  |  5 +++--
>  libavcodec/jpegxl_parser.c | 16 ++++++++--------
>  2 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/libavcodec/jpegxl_parse.c b/libavcodec/jpegxl_parse.c
> index 7cfdd3e7d5..022eed322d 100644
> --- a/libavcodec/jpegxl_parse.c
> +++ b/libavcodec/jpegxl_parse.c
> @@ -450,7 +450,8 @@ int ff_jpegxl_collect_codestream_header(const uint8_t *input_buffer, int input_l
>                                          uint8_t *buffer, int buflen, int *copied)
>  {
>      GetByteContext gb;
> -    int pos = 0, last_box = 0;
> +    int64_t pos = 0;
> +    int last_box = 0;
>      bytestream2_init(&gb, input_buffer, input_len);
>
>      while (1) {
> @@ -516,5 +517,5 @@ int ff_jpegxl_collect_codestream_header(const uint8_t *input_buffer, int input_l
>              break;
>      }
>
> -    return pos;
> +    return FFMIN(pos, INT_MAX);
>  }
> diff --git a/libavcodec/jpegxl_parser.c b/libavcodec/jpegxl_parser.c
> index a888e9ae6e..68404229a5 100644
> --- a/libavcodec/jpegxl_parser.c
> +++ b/libavcodec/jpegxl_parser.c
> @@ -155,12 +155,12 @@ typedef struct JXLParseContext {
>
>      /* using ISOBMFF-based container */
>      int container;
> -    int skip;
> +    int64_t skip;
>      int copied;
> -    int collected_size;
> -    int codestream_length;
> +    int64_t collected_size;
> +    int64_t codestream_length;
>      int skipped_icc;
> -    int next;
> +    int64_t next;
>
>      uint8_t cs_buffer[4096 + AV_INPUT_BUFFER_PADDING_SIZE];
>  } JXLParseContext;
> @@ -1396,7 +1396,7 @@ static int skip_boxes(JXLParseContext *ctx, const uint8_t *buf, int buf_size)
>      return 0;
>  }
>
> -static int try_parse(AVCodecParserContext *s, AVCodecContext *avctx, JXLParseContext *ctx,
> +static int64_t try_parse(AVCodecParserContext *s, AVCodecContext *avctx, JXLParseContext *ctx,
>                       const uint8_t *buf, int buf_size)
>  {
>      int ret, cs_buflen, header_skip;
> @@ -1489,10 +1489,10 @@ static int jpegxl_parse(AVCodecParserContext *s, AVCodecContext *avctx,
>      }
>
>      if ((!ctx->container || !ctx->codestream_length) && !ctx->next) {
> -        ret = try_parse(s, avctx, ctx, pbuf, pindex);
> -        if (ret < 0)
> +        int64_t ret64 = try_parse(s, avctx, ctx, pbuf, pindex);
> +        if (ret64 < 0)
>              goto flush;
> -        ctx->next = ret;
> +        ctx->next = ret64;
>          if (ctx->container)
>              ctx->skip += ctx->next;
>      }
> --
> 2.48.1
>

Works for me. Thanks.

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