diff mbox series

[FFmpeg-devel] avformat/mov: use an array of pointers for heif_item

Message ID 20241108234546.32631-1-jamrial@gmail.com
State New
Headers show
Series [FFmpeg-devel] avformat/mov: use an array of pointers for heif_item | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished

Commit Message

James Almer Nov. 8, 2024, 11:45 p.m. UTC
Pointers to specific entries in the array are stored in other structs, so
in the scenario where heif_item was reallocated when parsing an iloc box after
and iinf one, the pointers may end up referencing freed memory.

Fixes use-after-free with such samples.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavformat/isom.h |  2 +-
 libavformat/mov.c  | 75 ++++++++++++++++++++++++++++++----------------
 2 files changed, 51 insertions(+), 26 deletions(-)
diff mbox series

Patch

diff --git a/libavformat/isom.h b/libavformat/isom.h
index 49a4753fad..5aab5bad9b 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -360,7 +360,7 @@  typedef struct MOVContext {
     uint32_t max_stts_delta;
     int primary_item_id;
     int cur_item_id;
-    HEIFItem *heif_item;
+    HEIFItem **heif_item;
     int nb_heif_item;
     HEIFGrid *heif_grid;
     int nb_heif_grid;
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 8c3329b815..f9e5b9e199 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -197,10 +197,10 @@  static HEIFItem *heif_cur_item(MOVContext *c)
     HEIFItem *item = NULL;
 
     for (int i = 0; i < c->nb_heif_item; i++) {
-        if (c->heif_item[i].item_id != c->cur_item_id)
+        if (!c->heif_item[i] || c->heif_item[i]->item_id != c->cur_item_id)
             continue;
 
-        item = &c->heif_item[i];
+        item = c->heif_item[i];
         break;
     }
 
@@ -8657,7 +8657,7 @@  static int mov_read_idat(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 
 static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 {
-    HEIFItem *heif_item;
+    HEIFItem **heif_item;
     int version, offset_size, length_size, base_offset_size, index_size;
     int item_count, extent_count;
     int64_t base_offset, extent_offset, extent_length;
@@ -8688,12 +8688,13 @@  static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
         return AVERROR(ENOMEM);
     c->heif_item = heif_item;
     if (item_count > c->nb_heif_item)
-        memset(c->heif_item + c->nb_heif_item, 0,
+        memset(&c->heif_item[c->nb_heif_item], 0,
                sizeof(*c->heif_item) * (item_count - c->nb_heif_item));
     c->nb_heif_item = FFMAX(c->nb_heif_item, item_count);
 
     av_log(c->fc, AV_LOG_TRACE, "iloc: item_count %d\n", item_count);
     for (int i = 0; i < item_count; i++) {
+        HEIFItem *item = c->heif_item[i];
         int item_id = (version < 2) ? avio_rb16(pb) : avio_rb32(pb);
         int offset_type = (version > 0) ? avio_rb16(pb) & 0xf : 0;
 
@@ -8703,7 +8704,6 @@  static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
             avpriv_report_missing_feature(c->fc, "iloc offset type %d", offset_type);
             return AVERROR_PATCHWELCOME;
         }
-        c->heif_item[i].item_id = item_id;
 
         avio_rb16(pb);  // data_reference_index.
         if (rb_size(pb, &base_offset, base_offset_size) < 0)
@@ -8714,19 +8714,26 @@  static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
             avpriv_report_missing_feature(c->fc, "iloc: extent_count > 1");
             return AVERROR_PATCHWELCOME;
         }
-        for (int j = 0; j < extent_count; j++) {
+
             if (rb_size(pb, &extent_offset, offset_size) < 0 ||
                 rb_size(pb, &extent_length, length_size) < 0 ||
                 base_offset > INT64_MAX - extent_offset)
                 return AVERROR_INVALIDDATA;
+
+        if (!item)
+            item = c->heif_item[i] = av_mallocz(sizeof(*item));
+        if (!item)
+            return AVERROR(ENOMEM);
+
+        item->item_id = item_id;
+
             if (offset_type == 1)
-                c->heif_item[i].is_idat_relative = 1;
-            c->heif_item[i].extent_length = extent_length;
-            c->heif_item[i].extent_offset = base_offset + extent_offset;
+                item->is_idat_relative = 1;
+            item->extent_length = extent_length;
+            item->extent_offset = base_offset + extent_offset;
             av_log(c->fc, AV_LOG_TRACE, "iloc: item_idx %d, offset_type %d, "
                                         "extent_offset %"PRId64", extent_length %"PRId64"\n",
-                   i, offset_type, c->heif_item[i].extent_offset, c->heif_item[i].extent_length);
-        }
+                   i, offset_type, item->extent_offset, item->extent_length);
     }
 
     c->found_iloc = 1;
@@ -8735,6 +8742,7 @@  static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 
 static int mov_read_infe(MOVContext *c, AVIOContext *pb, MOVAtom atom, int idx)
 {
+    HEIFItem *item;
     AVBPrint item_name;
     int64_t size = atom.size;
     uint32_t item_type;
@@ -8774,15 +8782,21 @@  static int mov_read_infe(MOVContext *c, AVIOContext *pb, MOVAtom atom, int idx)
     if (size > 0)
         avio_skip(pb, size);
 
+    item = c->heif_item[idx];
+    if (!item)
+        item = c->heif_item[idx] = av_mallocz(sizeof(*item));
+    if (!item)
+        return AVERROR(ENOMEM);
+
     if (ret)
-        av_bprint_finalize(&item_name, &c->heif_item[idx].name);
-    c->heif_item[idx].item_id = item_id;
-    c->heif_item[idx].type    = item_type;
+        av_bprint_finalize(&item_name, &c->heif_item[idx]->name);
+    c->heif_item[idx]->item_id = item_id;
+    c->heif_item[idx]->type    = item_type;
 
     switch (item_type) {
     case MKTAG('a','v','0','1'):
     case MKTAG('h','v','c','1'):
-        ret = heif_add_stream(c, &c->heif_item[idx]);
+        ret = heif_add_stream(c, c->heif_item[idx]);
         if (ret < 0)
             return ret;
         break;
@@ -8793,7 +8807,7 @@  static int mov_read_infe(MOVContext *c, AVIOContext *pb, MOVAtom atom, int idx)
 
 static int mov_read_iinf(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 {
-    HEIFItem *heif_item;
+    HEIFItem **heif_item;
     int entry_count;
     int version, got_stream = 0, ret, i;
 
@@ -8811,7 +8825,7 @@  static int mov_read_iinf(MOVContext *c, AVIOContext *pb, MOVAtom atom)
         return AVERROR(ENOMEM);
     c->heif_item = heif_item;
     if (entry_count > c->nb_heif_item)
-        memset(c->heif_item + c->nb_heif_item, 0,
+        memset(&c->heif_item[c->nb_heif_item], 0,
                sizeof(*c->heif_item) * (entry_count - c->nb_heif_item));
     c->nb_heif_item = FFMAX(c->nb_heif_item, entry_count);
 
@@ -8833,7 +8847,10 @@  static int mov_read_iinf(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     return 0;
 fail:
     for (; i >= 0; i--) {
-        HEIFItem *item = &c->heif_item[i];
+        HEIFItem *item = c->heif_item[i];
+
+        if (!item)
+            continue;
 
         av_freep(&item->name);
         if (!item->st)
@@ -8861,9 +8878,9 @@  static int mov_read_iref_dimg(MOVContext *c, AVIOContext *pb, int version)
         }
     }
     for (int i = 0; i < c->nb_heif_item; i++) {
-        if (c->heif_item[i].item_id != from_item_id)
+        if (!c->heif_item[i] || c->heif_item[i]->item_id != from_item_id)
             continue;
-        item = &c->heif_item[i];
+        item = c->heif_item[i];
 
         switch (item->type) {
         case MKTAG('g','r','i','d'):
@@ -9779,8 +9796,9 @@  static int mov_read_close(AVFormatContext *s)
     av_freep(&mov->aes_decrypt);
     av_freep(&mov->chapter_tracks);
     for (i = 0; i < mov->nb_heif_item; i++) {
-        av_freep(&mov->heif_item[i].name);
-        av_freep(&mov->heif_item[i].icc_profile);
+        av_freep(&mov->heif_item[i]->name);
+        av_freep(&mov->heif_item[i]->icc_profile);
+        av_freep(&mov->heif_item[i]);
     }
     av_freep(&mov->heif_item);
     for (i = 0; i < mov->nb_heif_grid; i++) {
@@ -10164,9 +10182,13 @@  static int mov_parse_tiles(AVFormatContext *s)
             int k;
 
             for (k = 0; k < mov->nb_heif_item; k++) {
-                HEIFItem *item = &mov->heif_item[k];
-                AVStream *st = item->st;
+                HEIFItem *item = mov->heif_item[k];
+                AVStream *st;
 
+                if (!item)
+                    continue;
+
+                st = item->st;
                 if (item->item_id != tile_id)
                     continue;
                 if (!st) {
@@ -10233,11 +10255,14 @@  static int mov_parse_heif_items(AVFormatContext *s)
     int err;
 
     for (int i = 0; i < mov->nb_heif_item; i++) {
-        HEIFItem *item = &mov->heif_item[i];
+        HEIFItem *item = mov->heif_item[i];
         MOVStreamContext *sc;
         AVStream *st;
         int64_t offset = 0;
 
+        if (!item)
+            continue;
+
         if (!item->st) {
             if (item->item_id == mov->thmb_item_id) {
                 av_log(s, AV_LOG_ERROR, "HEIF thumbnail doesn't reference a stream\n");