From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.ffmpeg.org (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTPS id 321AA4B16F for ; Thu, 21 Aug 2025 17:22:01 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTP id CD36068D7E4; Thu, 21 Aug 2025 20:21:57 +0300 (EEST) Received: from c1ad6a1ecdc3 (code.ffmpeg.org [188.245.149.3]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTPS id B512568D71E for ; Thu, 21 Aug 2025 20:21:56 +0300 (EEST) MIME-Version: 1.0 To: ffmpeg-devel@ffmpeg.org Subject: [FFmpeg-devel] =?utf-8?q?=5BPATCH=5D_ff-tmp-sanm-unhack_=28PR_?= =?utf-8?q?=2320306=29?= X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: michaelni via ffmpeg-devel Reply-To: FFmpeg development discussions and patches Cc: michaelni Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Message-Id: <20250821172157.CD36068D7E4@ffbox0-bg.ffmpeg.org> Date: Thu, 21 Aug 2025 20:21:57 +0300 (EEST) Archived-At: List-Archive: List-Post: PR #20306 opened by michaelni URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20306 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20306.patch Remove some pointers into freed buffers Fixes BIGSLEEP-440183164 >From fbc1d9dca1aa30ac5e2c63c295652c4b1f54d7be Mon Sep 17 00:00:00 2001 From: Michael Niedermayer Date: Thu, 21 Aug 2025 18:40:26 +0200 Subject: [PATCH 1/3] avcodec/sanm: Replace impossible bitstream check by assert the space left and size have already been cross checked by the caller Signed-off-by: Michael Niedermayer --- libavcodec/sanm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c index e4308af647..1495da2a1e 100644 --- a/libavcodec/sanm.c +++ b/libavcodec/sanm.c @@ -1832,8 +1832,7 @@ static int process_ftch(SANMVideoContext *ctx, int size) xoff = bytestream2_get_le16u(&ctx->gb); yoff = bytestream2_get_le16u(&ctx->gb); } else { - if (bytestream2_get_bytes_left(&ctx->gb) < 12) - return AVERROR_INVALIDDATA; + av_assert0(bytestream2_get_bytes_left(&ctx->gb) >= 12); bytestream2_skip(&ctx->gb, 4); xoff = bytestream2_get_be32u(&ctx->gb); yoff = bytestream2_get_be32u(&ctx->gb); -- 2.49.1 >From e7b08e87c6f5205bc4c6355ab7138a2ae0a8de17 Mon Sep 17 00:00:00 2001 From: Michael Niedermayer Date: Thu, 21 Aug 2025 18:59:36 +0200 Subject: [PATCH 2/3] avcodec/sanm: Remove left/top adjustment hack Fixes: write after free Fixes: BIGSLEEP-440183164/process_ftch.anim Found-by: Google Big Sleep Signed-off-by: Michael Niedermayer --- libavcodec/sanm.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c index 1495da2a1e..83e0eb7241 100644 --- a/libavcodec/sanm.c +++ b/libavcodec/sanm.c @@ -1660,7 +1660,7 @@ static int old_codec48(SANMVideoContext *ctx, int width, int height) return 0; } -static int process_frame_obj(SANMVideoContext *ctx, GetByteContext *gb) +static int process_frame_obj(SANMVideoContext *ctx, GetByteContext *gb, int extraleft, int extratop) { uint16_t w, h, parm2; uint8_t codec, param; @@ -1669,8 +1669,8 @@ static int process_frame_obj(SANMVideoContext *ctx, GetByteContext *gb) codec = bytestream2_get_byteu(gb); param = bytestream2_get_byteu(gb); - left = bytestream2_get_le16u(gb); - top = bytestream2_get_le16u(gb); + left = bytestream2_get_le16u(gb) + extraleft; + top = bytestream2_get_le16u(gb) + extratop; w = bytestream2_get_le16u(gb); h = bytestream2_get_le16u(gb); bytestream2_skip(gb, 2); @@ -1820,7 +1820,7 @@ static int process_frame_obj(SANMVideoContext *ctx, GetByteContext *gb) static int process_ftch(SANMVideoContext *ctx, int size) { uint8_t *sf = ctx->stored_frame; - int xoff, yoff, left, top, ret; + int xoff, yoff, ret; GetByteContext gb; uint32_t sz; @@ -1841,18 +1841,10 @@ static int process_ftch(SANMVideoContext *ctx, int size) sz = *(uint32_t *)(sf + 0); if ((sz > 0) && (sz <= ctx->stored_frame_size - 4)) { /* add the FTCH offsets to the left/top values of the stored FOBJ */ - left = av_le2ne16(*(int16_t *)(sf + 4 + 2)); - top = av_le2ne16(*(int16_t *)(sf + 4 + 4)); - *(int16_t *)(sf + 4 + 2) = av_le2ne16(left + xoff); - *(int16_t *)(sf + 4 + 4) = av_le2ne16(top + yoff); /* decode the stored FOBJ */ bytestream2_init(&gb, sf + 4, sz); - ret = process_frame_obj(ctx, &gb); - - /* now restore the original left/top values again */ - *(int16_t *)(sf + 4 + 2) = av_le2ne16(left); - *(int16_t *)(sf + 4 + 4) = av_le2ne16(top); + ret = process_frame_obj(ctx, &gb, xoff, yoff); } else { /* this happens a lot in RA1: The individual files are meant to * be played in sequence, with some referencing objects STORed @@ -2359,7 +2351,7 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *frame, case MKBETAG('F', 'O', 'B', 'J'): if (size < 16) return AVERROR_INVALIDDATA; - if (ret = process_frame_obj(ctx, &ctx->gb)) + if (ret = process_frame_obj(ctx, &ctx->gb, 0, 0)) return ret; have_img = 1; -- 2.49.1 >From 36ca6798358686268093473cbbcb3e706d8ad2c2 Mon Sep 17 00:00:00 2001 From: Michael Niedermayer Date: Thu, 21 Aug 2025 19:06:03 +0200 Subject: [PATCH 3/3] avcodec/sanm: Eliminate reference into reallocated frame AFAIK the original decoder uses the frame buffers in very strange ways our implementation seems to mimic that and that results in the bitstream input to point into a frame buffer while code then parses that and potentially reallocates the frame buffer leaving pointers hanging into dealllocated space This simply uses a temporary buffer Fixes: Writing into freed buffers Fixes: BIGSLEEP-440183164/old_codec21.anim Fixes: BIGSLEEP-440183164/old_codec4.anim Found-by: Google Big Sleep Signed-off-by: Michael Niedermayer --- libavcodec/sanm.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c index 83e0eb7241..b1a0d05ddc 100644 --- a/libavcodec/sanm.c +++ b/libavcodec/sanm.c @@ -1843,8 +1843,13 @@ static int process_ftch(SANMVideoContext *ctx, int size) /* add the FTCH offsets to the left/top values of the stored FOBJ */ /* decode the stored FOBJ */ - bytestream2_init(&gb, sf + 4, sz); + uint8_t *bitstream = av_malloc(sz + AV_INPUT_BUFFER_PADDING_SIZE); + if (!bitstream) + return AVERROR(ENOMEM); + memcpy(bitstream, sf + 4, sz); + bytestream2_init(&gb, bitstream, sz); ret = process_frame_obj(ctx, &gb, xoff, yoff); + av_free(bitstream); } else { /* this happens a lot in RA1: The individual files are meant to * be played in sequence, with some referencing objects STORed -- 2.49.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".