[FFmpeg-devel] [PATCH] ff-tmp-sanm-unhack (PR #20306)

[michaelni via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>]

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 <michael@niedermayer.cc>
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 <michael@niedermayer.cc>
---
 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 <michael@niedermayer.cc>
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 <michael@niedermayer.cc>
---
 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 <michael@niedermayer.cc>
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 <michael@niedermayer.cc>
---
 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".