This makes me feel we should probably just rely on mfra

Spotify comments
----------------
None

/Tomas
From c3adfabb5ad565f562c6a187c2fa2913755d4345 Mon Sep 17 00:00:00 2001
From: Mattias Wadman <wa...@spotify.com>
Date: Fri, 26 May 2023 14:16:58 +0200
Subject: [PATCH 13/15] avformat/mov: Add more moov and moov child heuristic
 checks

Fixes problem seeking backwards and probably also infinite seek loop.
Make sure size is at leats size of header and not beyond end of file.
Also handle more IO-errors.

GOL-1423: a file fails in MAL
---
 libavformat/mov.c | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 89e6633bfa..6ab5232dab 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -1581,14 +1581,17 @@ static int mov_read_moov(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 }
 
 static int mov_read_atom_header(AVIOContext *pb, MOVAtom *atom) {
+    int header_size = 8;
     atom->size = avio_rb32(pb);
     atom->type = avio_rl32(pb);
     if (atom->size == 1) {
         atom->size = avio_rb64(pb);
-        return 12;
-    } else {
-        return 8;
+        header_size += 8;
+    }
+    if (pb->error) {
+        return pb->error;
     }
+    return header_size;
 }
 
 static int mov_try_read_moov(MOVContext *c, AVIOContext *pb, int64_t offset, int64_t filesize) {
@@ -1608,7 +1611,8 @@ static int mov_try_read_moov(MOVContext *c, AVIOContext *pb, int64_t offset, int
         return moov_header_size;
     }
 
-    if (moov.type != MKTAG('m','o','o','v')) {
+    if (moov.size < moov_header_size || moov.size > (filesize-offset) ||
+        moov.type != MKTAG('m','o','o','v')) {
         av_log(c->fc, AV_LOG_TRACE,
             "Moov-hint at %"PRId64" failed type&size-check, %"PRId64" != %"PRId64"\n",
             offset, moov.size, filesize - offset
@@ -1618,13 +1622,30 @@ static int mov_try_read_moov(MOVContext *c, AVIOContext *pb, int64_t offset, int
 
     /* Iterate through inner boxes, looking for some must-have types*/
     while (avio_tell(pb) < offset + moov.size) {
-        ret = mov_read_atom_header(pb, &child);
+        int64_t header_size;
+        if ((header_size = mov_read_atom_header(pb, &child)) < 0) {
+            return header_size;
+        }
+
+        if (child.size < header_size || child.size-header_size > (filesize-avio_tell(pb))) {
+            av_log(c->fc, AV_LOG_TRACE,
+                "Moov-hint child %s at %"PRId64" failed size-check %"PRId64"\n",
+                av_fourcc2str(child.type), avio_tell(pb), child.size
+            );
+            return -1;
+        }
         for (i=0; i < num_boxes; i++) {
             if (child.type == boxes_seen[i].type) {
                 boxes_seen[i].seen = 1;
             }
         }
-        avio_seek(pb, child.size - ret, SEEK_CUR);
+        ret = avio_seek(pb, child.size - header_size, SEEK_CUR);
+        if (ret < 0) {
+            av_log(c->fc, AV_LOG_TRACE, "avio_seek() failed for child %s of size %"PRId64"\n",
+                av_fourcc2str(child.type), (int64_t)child.size
+            );
+            return ret;
+        }
     }
 
     /* Verify that expected children were seen */
@@ -1640,7 +1661,9 @@ static int mov_try_read_moov(MOVContext *c, AVIOContext *pb, int64_t offset, int
 
     /* Verify that the remaining root-level boxes ends up evenly at the end */
     while (avio_tell(pb) < filesize) {
-        ret = mov_read_atom_header(pb, &child);
+        if ((ret = mov_read_atom_header(pb, &child)) < 0) {
+            return ret;
+        }
         for (i=0; i < num_boxes; i++) {
             if (child.type == boxes_seen[i].type) {
                 boxes_seen[i].seen = 1;
-- 
2.39.2

_______________________________________________
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