[FFmpeg-devel] [PATCH] avcodec/sanm: minor fixes (PR #20997)

[Manuel Lauss via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>]

PR #20997 opened by Manuel Lauss (mlauss2)
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20997
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20997.patch

- Do not consider dimenions of codec37/47/48 frames as trustworthy.  Esp with Making Magic where the codec48 is smaller and embedded onto a larger carnvas.
- Throw away codec37/47/48 frames that are larger than the current canvas.  It's also what the original implementations do.
- in decode_init() there's no reason not to accept a canvas size even for ANIM.  While the SAN files do not provide any image dimensions in their headers, there's no reason to reject a known canvas size when the decoder is initialilzed.
- in process_frame_obj(), apply the additional xoff/yoff after frame size determination, as to not disturb the frame size with the additional fobj offset.

Tested with all my test videos and has also had ~20 hours of fuzzing as well.


>From 608197d882047bb9ef81fac16f9925cd03e023fd Mon Sep 17 00:00:00 2001
From: Manuel Lauss <manuel.lauss@gmail.com>
Date: Wed, 19 Nov 2025 12:21:41 +0100
Subject: [PATCH 1/4] avcodec/sanm: fobj: do not use codec-id to determine
 canvas size.

Codec>=37 with smaller dimensions can be embedded onto larger canvasses;
it makes no sense to trust their dimensions explicitly.

Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
---
 libavcodec/sanm.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c
index 771ecf8246..3fc52f48b3 100644
--- a/libavcodec/sanm.c
+++ b/libavcodec/sanm.c
@@ -2057,11 +2057,6 @@ static int process_frame_obj(SANMVideoContext *ctx, GetByteContext *gb,
             if (w > xres || h > yres)
                 return AVERROR_INVALIDDATA;
             ctx->have_dimensions = 1;
-        } else if (fsc) {
-            /* these codecs work on full frames, trust their dimensions */
-            xres = w;
-            yres = h;
-            ctx->have_dimensions = 1;
         } else {
             /* detect common sizes */
             xres = w + left;
-- 
2.49.1


>From de1c66ce72b968fc60820d8d37c94dd0f9b6f89f Mon Sep 17 00:00:00 2001
From: Manuel Lauss <manuel.lauss@gmail.com>
Date: Wed, 19 Nov 2025 13:00:41 +0100
Subject: [PATCH 2/4] avcodec/sanm: fobj codec37+: reject too large frames.

For the diff-buffer codecs, outright reject frames that are larger
than the currently configured canvas.  This is also what the DOS
players do.

Gets also rid of an artifact in the "sq1.san" Intro video of "The Dig".

Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
---
 libavcodec/sanm.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c
index 3fc52f48b3..68c80954ff 100644
--- a/libavcodec/sanm.c
+++ b/libavcodec/sanm.c
@@ -2087,17 +2087,9 @@ static int process_frame_obj(SANMVideoContext *ctx, GetByteContext *gb,
             }
         }
     } else {
-        if (((w > ctx->width) || (h > ctx->height) || (w * h > ctx->buf_size)) && fsc) {
-            /* correct unexpected overly large frames: this happens
-             * for instance with The Dig's sq1.san video: it has a few
-             * (all black) 640x480 frames halfway in, while the rest is
-             * 320x200.
-             */
-            av_log(ctx->avctx, AV_LOG_WARNING,
-                   "resizing too large fobj: c%d  %d %d @ %d %d\n", codec, w, h, left, top);
-            w = ctx->width;
-            h = ctx->height;
-        }
+        /* for codec37/47/48, reject too large frames, like the DOS players do. */
+        if (((w > ctx->width) || (h > ctx->height) || (w * h > ctx->buf_size)) && fsc)
+            return AVERROR_INVALIDDATA;
     }
 
     /* users of codecs>=37 are subversion 2, enforce that for STOR/FTCH */
-- 
2.49.1


>From 2a2185b262849c25b5b59744f3860832032184fa Mon Sep 17 00:00:00 2001
From: Manuel Lauss <manuel.lauss@gmail.com>
Date: Wed, 19 Nov 2025 13:25:01 +0100
Subject: [PATCH 3/4] avcodec/sanm: accept fixed dimensions for ANIM at
 decode_init

This undoes 556cef27d905d31, which I added to fix a fuzzer-crash,
but there's no reason to expect the decoder can only be invoked
via the smush demuxer.  Instead also accept a range of dimensions
from 2x2 up to 640x480.

Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
---
 libavcodec/sanm.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c
index 68c80954ff..2621378567 100644
--- a/libavcodec/sanm.c
+++ b/libavcodec/sanm.c
@@ -636,9 +636,14 @@ static av_cold int decode_init(AVCodecContext *avctx)
     avctx->pix_fmt = ctx->version ? AV_PIX_FMT_RGB565 : AV_PIX_FMT_PAL8;
 
     if (!ctx->version) {
-        // ANIM has no dimensions in the header, distrust the incoming data.
-        avctx->width = avctx->height = 0;
-        ctx->have_dimensions = 0;
+        // ANIM valid range is 2x2 up to 640x480
+        if (avctx->width != 0 && avctx->height != 0) {
+            if (avctx->width > 640 || avctx->height > 480 ||
+                avctx->width < 2 || avctx->height < 2) {
+                return AVERROR_INVALIDDATA;
+            }
+            ctx->have_dimensions = 1;
+        }
     } else if (avctx->width > 800 || avctx->height > 600 ||
                avctx->width < 8 || avctx->height < 8) {
         // BL16 valid range is 8x8 - 800x600
-- 
2.49.1


>From 8a865c555e75c70f1bf8e7ee96271dd15f6d62c9 Mon Sep 17 00:00:00 2001
From: Manuel Lauss <manuel.lauss@gmail.com>
Date: Wed, 19 Nov 2025 17:01:18 +0100
Subject: [PATCH 4/4] avcodec/sanm: fobj: apply the x/y offsets after size
 determination

Otherwise a wrong size might be determined.

Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
---
 libavcodec/sanm.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c
index 2621378567..c4cdfa52ca 100644
--- a/libavcodec/sanm.c
+++ b/libavcodec/sanm.c
@@ -2023,8 +2023,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) + xoff;
-    top   = bytestream2_get_le16u(gb) + yoff;
+    left  = bytestream2_get_le16u(gb);
+    top   = bytestream2_get_le16u(gb);
     w     = bytestream2_get_le16u(gb);
     h     = bytestream2_get_le16u(gb);
     bytestream2_skip(gb, 2);
@@ -2110,6 +2110,9 @@ static int process_frame_obj(SANMVideoContext *ctx, GetByteContext *gb,
             memset(ctx->fbuf, 0, ctx->frm0_size);
     }
 
+    left += xoff;
+    top += yoff;
+
     switch (codec) {
     case 1:
     case 3:
-- 
2.49.1

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org