From 9143ab0e5a75519c899cae2996d07b3f69bcfb24 Mon Sep 17 00:00:00 2001 From: Dale Curtis Date: Tue, 6 Jan 2015 04:00:43 +0100 Subject: [PATCH] mov: Fix overflow and error handling in read_tfra(). Under abnormal conditions the item_count may exceed the max allocation size on 32-bit systems, this causes the allocated size to overflow and become too small for the given count. Additionally, if av_reallocp() fails its allocation, the fragment_index_count is not correctly decremented. Ensuring further havoc may be wrought, the error code for read_tfra() is not checked upon return. Found-by: Paul Mehta positive return code and use of _array functions by commiter Signed-off-by: Michael Niedermayer (cherry picked from commit db42d93a61be26873be6115c57f5921b4dfdec14) Signed-off-by: Michael Niedermayer --- libavformat/mov.c | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/libavformat/mov.c b/libavformat/mov.c index b888c6736d..3a93897a96 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -3782,35 +3782,39 @@ static void export_orphan_timecode(AVFormatContext *s) static int read_tfra(MOVContext *mov, AVIOContext *f) { MOVFragmentIndex* index = NULL; - int version, fieldlength, i, j, err; + int version, fieldlength, i, j; int64_t pos = avio_tell(f); uint32_t size = avio_rb32(f); + void *tmp; + if (avio_rb32(f) != MKBETAG('t', 'f', 'r', 'a')) { - return -1; + return 1; } av_log(mov->fc, AV_LOG_VERBOSE, "found tfra\n"); index = av_mallocz(sizeof(MOVFragmentIndex)); if (!index) { return AVERROR(ENOMEM); } - mov->fragment_index_count++; - if ((err = av_reallocp(&mov->fragment_index_data, - mov->fragment_index_count * - sizeof(MOVFragmentIndex*))) < 0) { + + tmp = av_realloc_array(mov->fragment_index_data, + mov->fragment_index_count + 1, + sizeof(MOVFragmentIndex*)); + if (!tmp) { av_freep(&index); - return err; + return AVERROR(ENOMEM); } - mov->fragment_index_data[mov->fragment_index_count - 1] = - index; + mov->fragment_index_data = tmp; + mov->fragment_index_data[mov->fragment_index_count++] = index; version = avio_r8(f); avio_rb24(f); index->track_id = avio_rb32(f); fieldlength = avio_rb32(f); index->item_count = avio_rb32(f); - index->items = av_mallocz( - index->item_count * sizeof(MOVFragmentIndexItem)); + index->items = av_mallocz_array( + index->item_count, sizeof(MOVFragmentIndexItem)); if (!index->items) { + index->item_count = 0; return AVERROR(ENOMEM); } for (i = 0; i < index->item_count; i++) { @@ -3864,11 +3868,13 @@ static int mov_read_mfra(MOVContext *c, AVIOContext *f) av_log(c->fc, AV_LOG_DEBUG, "doesn't look like mfra (tag mismatch)\n"); goto fail; } - ret = 0; av_log(c->fc, AV_LOG_VERBOSE, "stream has mfra\n"); - while (!read_tfra(c, f)) { - /* Empty */ - } + do { + ret = read_tfra(c, f); + if (ret < 0) + goto fail; + } while (!ret); + ret = 0; fail: seek_ret = avio_seek(f, original_pos, SEEK_SET); if (seek_ret < 0) {