On Mon, Feb 5, 2024 at 12:07 PM Michael Niedermayer <mich...@niedermayer.cc>
wrote:

> assuming atom.size is an arbitrary 64bit value
> then the value of FFMIN() is also 64bit but entries is unsigned 32bit,
> this truncation
> would allow setting entries to values outside whats expected from FFMIN()
> also we seem to disalllow entries == 0 before this
> and its maybe possible to set entries = 0 here, bypassing the == 0 check
> before


Thanks. I've moved the clamp up to before the zero check. The only way a
bad 64-bit value could get in is if atom.size < 8, which I didn't think was
possible, but I've added a FFMAX(0,) there too.

- dale
From db3e9ffc364cc94cb3a72696d4d4858af6abcc42 Mon Sep 17 00:00:00 2001
From: Dale Curtis <dalecurtis@chromium.org>
Date: Fri, 2 Feb 2024 20:49:44 +0000
Subject: [PATCH] [mov] Avoid OOM for invalid STCO / CO64 constructions.

The `entries` value is read directly from the stream and used to
allocate memory. This change clamps `entries` to however many are
possible in the remaining atom or file size (whichever is smallest).

Fixes https://crbug.com/1429357

Signed-off-by: Dale Curtis <dalecurtis@chromium.org>
---
 libavformat/mov.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index af95e1f662..1e4850fe9f 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -2228,7 +2228,12 @@ static int mov_read_stco(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     avio_r8(pb); /* version */
     avio_rb24(pb); /* flags */
 
-    entries = avio_rb32(pb);
+    // Clamp allocation size for `chunk_offsets` -- don't throw an error for an
+    // invalid count since the EOF path doesn't throw either.
+    entries =
+        FFMIN(avio_rb32(pb),
+              FFMAX(0, (atom.size - 8) /
+                           (atom.type == MKTAG('s', 't', 'c', 'o') ? 4 : 8)));
 
     if (!entries)
         return 0;
@@ -2237,6 +2242,7 @@ static int mov_read_stco(MOVContext *c, AVIOContext *pb, MOVAtom atom)
         av_log(c->fc, AV_LOG_WARNING, "Ignoring duplicated STCO atom\n");
         return 0;
     }
+
     av_free(sc->chunk_offsets);
     sc->chunk_count = 0;
     sc->chunk_offsets = av_malloc_array(entries, sizeof(*sc->chunk_offsets));
-- 
2.44.0.rc0.258.g7320e95886-goog

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to