avformat/mov: use an array of pointers for heif_item

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>
This commit is contained in:
James Almer 2024-11-08 20:43:39 -03:00
parent 23697c3f02
commit f8fcebae95
2 changed files with 60 additions and 35 deletions

View File

@ -355,7 +355,7 @@ typedef struct MOVContext {
uint32_t max_stts_delta; uint32_t max_stts_delta;
int primary_item_id; int primary_item_id;
int cur_item_id; int cur_item_id;
HEIFItem *heif_item; HEIFItem **heif_item;
int nb_heif_item; int nb_heif_item;
HEIFGrid *heif_grid; HEIFGrid *heif_grid;
int nb_heif_grid; int nb_heif_grid;

View File

@ -196,10 +196,10 @@ static HEIFItem *heif_cur_item(MOVContext *c)
HEIFItem *item = NULL; HEIFItem *item = NULL;
for (int i = 0; i < c->nb_heif_item; i++) { 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; continue;
item = &c->heif_item[i]; item = c->heif_item[i];
break; break;
} }
@ -8612,7 +8612,7 @@ static int mov_read_idat(MOVContext *c, AVIOContext *pb, MOVAtom atom)
static int mov_read_iloc(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 version, offset_size, length_size, base_offset_size, index_size;
int item_count, extent_count; int item_count, extent_count;
int64_t base_offset, extent_offset, extent_length; int64_t base_offset, extent_offset, extent_length;
@ -8643,12 +8643,13 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
return AVERROR(ENOMEM); return AVERROR(ENOMEM);
c->heif_item = heif_item; c->heif_item = heif_item;
if (item_count > c->nb_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)); sizeof(*c->heif_item) * (item_count - c->nb_heif_item));
c->nb_heif_item = FFMAX(c->nb_heif_item, item_count); 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); av_log(c->fc, AV_LOG_TRACE, "iloc: item_count %d\n", item_count);
for (int i = 0; i < item_count; i++) { 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 item_id = (version < 2) ? avio_rb16(pb) : avio_rb32(pb);
int offset_type = (version > 0) ? avio_rb16(pb) & 0xf : 0; int offset_type = (version > 0) ? avio_rb16(pb) & 0xf : 0;
@ -8658,7 +8659,6 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
avpriv_report_missing_feature(c->fc, "iloc offset type %d", offset_type); avpriv_report_missing_feature(c->fc, "iloc offset type %d", offset_type);
return AVERROR_PATCHWELCOME; return AVERROR_PATCHWELCOME;
} }
c->heif_item[i].item_id = item_id;
avio_rb16(pb); // data_reference_index. avio_rb16(pb); // data_reference_index.
if (rb_size(pb, &base_offset, base_offset_size) < 0) if (rb_size(pb, &base_offset, base_offset_size) < 0)
@ -8669,19 +8669,26 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
avpriv_report_missing_feature(c->fc, "iloc: extent_count > 1"); avpriv_report_missing_feature(c->fc, "iloc: extent_count > 1");
return AVERROR_PATCHWELCOME; return AVERROR_PATCHWELCOME;
} }
for (int j = 0; j < extent_count; j++) {
if (rb_size(pb, &extent_offset, offset_size) < 0 || if (rb_size(pb, &extent_offset, offset_size) < 0 ||
rb_size(pb, &extent_length, length_size) < 0 || rb_size(pb, &extent_length, length_size) < 0 ||
base_offset > INT64_MAX - extent_offset) base_offset > INT64_MAX - extent_offset)
return AVERROR_INVALIDDATA; 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) if (offset_type == 1)
c->heif_item[i].is_idat_relative = 1; item->is_idat_relative = 1;
c->heif_item[i].extent_length = extent_length; item->extent_length = extent_length;
c->heif_item[i].extent_offset = base_offset + extent_offset; item->extent_offset = base_offset + extent_offset;
av_log(c->fc, AV_LOG_TRACE, "iloc: item_idx %d, offset_type %d, " av_log(c->fc, AV_LOG_TRACE, "iloc: item_idx %d, offset_type %d, "
"extent_offset %"PRId64", extent_length %"PRId64"\n", "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; c->found_iloc = 1;
@ -8690,6 +8697,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) static int mov_read_infe(MOVContext *c, AVIOContext *pb, MOVAtom atom, int idx)
{ {
HEIFItem *item;
AVBPrint item_name; AVBPrint item_name;
int64_t size = atom.size; int64_t size = atom.size;
uint32_t item_type; uint32_t item_type;
@ -8729,15 +8737,21 @@ static int mov_read_infe(MOVContext *c, AVIOContext *pb, MOVAtom atom, int idx)
if (size > 0) if (size > 0)
avio_skip(pb, size); 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) if (ret)
av_bprint_finalize(&item_name, &c->heif_item[idx].name); av_bprint_finalize(&item_name, &c->heif_item[idx]->name);
c->heif_item[idx].item_id = item_id; c->heif_item[idx]->item_id = item_id;
c->heif_item[idx].type = item_type; c->heif_item[idx]->type = item_type;
switch (item_type) { switch (item_type) {
case MKTAG('a','v','0','1'): case MKTAG('a','v','0','1'):
case MKTAG('h','v','c','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) if (ret < 0)
return ret; return ret;
break; break;
@ -8748,7 +8762,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) static int mov_read_iinf(MOVContext *c, AVIOContext *pb, MOVAtom atom)
{ {
HEIFItem *heif_item; HEIFItem **heif_item;
int entry_count; int entry_count;
int version, got_stream = 0, ret, i; int version, got_stream = 0, ret, i;
@ -8766,15 +8780,17 @@ static int mov_read_iinf(MOVContext *c, AVIOContext *pb, MOVAtom atom)
return AVERROR(ENOMEM); return AVERROR(ENOMEM);
c->heif_item = heif_item; c->heif_item = heif_item;
if (entry_count > c->nb_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)); sizeof(*c->heif_item) * (entry_count - c->nb_heif_item));
c->nb_heif_item = FFMAX(c->nb_heif_item, entry_count); c->nb_heif_item = FFMAX(c->nb_heif_item, entry_count);
for (i = 0; i < entry_count; i++) { for (i = 0; i < entry_count; i++) {
MOVAtom infe; MOVAtom infe;
if (avio_feof(pb)) if (avio_feof(pb)) {
return AVERROR_INVALIDDATA; ret = AVERROR_INVALIDDATA;
goto fail;
}
infe.size = avio_rb32(pb) - 8; infe.size = avio_rb32(pb) - 8;
infe.type = avio_rl32(pb); infe.type = avio_rl32(pb);
ret = mov_read_infe(c, pb, infe, i); ret = mov_read_infe(c, pb, infe, i);
@ -8788,7 +8804,10 @@ static int mov_read_iinf(MOVContext *c, AVIOContext *pb, MOVAtom atom)
return 0; return 0;
fail: fail:
for (; i >= 0; i--) { for (; i >= 0; i--) {
HEIFItem *item = &c->heif_item[i]; HEIFItem *item = c->heif_item[i];
if (!item)
continue;
av_freep(&item->name); av_freep(&item->name);
if (!item->st) if (!item->st)
@ -8816,9 +8835,9 @@ static int mov_read_iref_dimg(MOVContext *c, AVIOContext *pb, int version)
} }
} }
for (int i = 0; i < c->nb_heif_item; i++) { 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; continue;
item = &c->heif_item[i]; item = c->heif_item[i];
switch (item->type) { switch (item->type) {
case MKTAG('g','r','i','d'): case MKTAG('g','r','i','d'):
@ -9692,8 +9711,12 @@ static int mov_read_close(AVFormatContext *s)
av_freep(&mov->aes_decrypt); av_freep(&mov->aes_decrypt);
av_freep(&mov->chapter_tracks); av_freep(&mov->chapter_tracks);
for (i = 0; i < mov->nb_heif_item; i++) for (i = 0; i < mov->nb_heif_item; i++) {
av_freep(&mov->heif_item[i].name); if (!mov->heif_item[i])
continue;
av_freep(&mov->heif_item[i]->name);
av_freep(&mov->heif_item[i]);
}
av_freep(&mov->heif_item); av_freep(&mov->heif_item);
for (i = 0; i < mov->nb_heif_grid; i++) { for (i = 0; i < mov->nb_heif_grid; i++) {
av_freep(&mov->heif_grid[i].tile_id_list); av_freep(&mov->heif_grid[i].tile_id_list);
@ -10009,10 +10032,10 @@ static int mov_parse_tiles(AVFormatContext *s)
int k; int k;
for (k = 0; k < mov->nb_heif_item; k++) { for (k = 0; k < mov->nb_heif_item; k++) {
HEIFItem *item = &mov->heif_item[k]; HEIFItem *item = mov->heif_item[k];
AVStream *st = item->st; AVStream *st = item->st;
if (item->item_id != tile_id) if (!item || item->item_id != tile_id)
continue; continue;
if (!st) { if (!st) {
av_log(s, AV_LOG_WARNING, "HEIF item id %d from grid id %d doesn't " av_log(s, AV_LOG_WARNING, "HEIF item id %d from grid id %d doesn't "
@ -10078,11 +10101,13 @@ static int mov_parse_heif_items(AVFormatContext *s)
int err; int err;
for (int i = 0; i < mov->nb_heif_item; i++) { for (int i = 0; i < mov->nb_heif_item; i++) {
HEIFItem *item = &mov->heif_item[i]; HEIFItem *item = mov->heif_item[i];
MOVStreamContext *sc; MOVStreamContext *sc;
AVStream *st; AVStream *st;
int64_t offset = 0; int64_t offset = 0;
if (!item)
continue;
if (!item->st) { if (!item->st) {
if (item->item_id == mov->thmb_item_id) { if (item->item_id == mov->thmb_item_id) {
av_log(s, AV_LOG_ERROR, "HEIF thumbnail doesn't reference a stream\n"); av_log(s, AV_LOG_ERROR, "HEIF thumbnail doesn't reference a stream\n");