[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 at 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 at gmail.com>
> Reported-by: Kacper Michajlow <kasper93 at 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