diff mbox series

[FFmpeg-devel,13/15] avformat/mov: Add more moov and moov child heuristic checks

Message ID a8d92b1817d1d719db5ccf37f5d6488bb966a8b9.camel@haerdin.se
State New
Headers show
Series Spotify patchset | expand

Commit Message

Tomas Härdin Oct. 29, 2024, 2:50 p.m. UTC
This makes me feel we should probably just rely on mfra

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

/Tomas
diff mbox series

Patch

From c3adfabb5ad565f562c6a187c2fa2913755d4345 Mon Sep 17 00:00:00 2001
From: Mattias Wadman <wader@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