avformat/matroskadec: Fix heap-buffer overflow upon gigantic timestamps

The WebM DASH Manifest demuxer creates a comma-delimited list of
all the timestamps of index entries. It allocates 20 bytes per
timestamp; yet the largest 64bit numbers have 20 decimal digits
(for int64_t it can be '-'+ 19 digits), so that one needs 21B
per entry because of the comma (resp. the final NUL).

The code uses snprintf, but snprintf returns the strlen of the string
that would have been written had the supplied buffer been big enough.
And if this is 21, then the next entry is written at an offset of 21
from the current position. So if enough such entries exist, the buffer
won't suffice.

This commit fixes this by replacing the allocation of buffer for
the supposedly worst-case with dynamic allocations by using an AVBPrint.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
This commit is contained in:
Andreas Rheinhardt 2021-08-25 16:13:12 +02:00
parent 7352c370fa
commit b2d61d0f02
1 changed files with 19 additions and 16 deletions

View File

@ -35,6 +35,7 @@
#include "libavutil/avstring.h" #include "libavutil/avstring.h"
#include "libavutil/base64.h" #include "libavutil/base64.h"
#include "libavutil/bprint.h"
#include "libavutil/dict.h" #include "libavutil/dict.h"
#include "libavutil/intfloat.h" #include "libavutil/intfloat.h"
#include "libavutil/intreadwrite.h" #include "libavutil/intreadwrite.h"
@ -4146,10 +4147,12 @@ static int webm_dash_manifest_cues(AVFormatContext *s, int64_t init_range)
MatroskaDemuxContext *matroska = s->priv_data; MatroskaDemuxContext *matroska = s->priv_data;
EbmlList *seekhead_list = &matroska->seekhead; EbmlList *seekhead_list = &matroska->seekhead;
MatroskaSeekhead *seekhead = seekhead_list->elem; MatroskaSeekhead *seekhead = seekhead_list->elem;
AVStream *const st = s->streams[0];
AVBPrint bprint;
char *buf; char *buf;
int64_t cues_start = -1, cues_end = -1, before_pos, bandwidth; int64_t cues_start = -1, cues_end = -1, before_pos, bandwidth;
int i; int i;
int end = 0; int ret;
// determine cues start and end positions // determine cues start and end positions
for (i = 0; i < seekhead_list->nb_elem; i++) for (i = 0; i < seekhead_list->nb_elem; i++)
@ -4180,6 +4183,9 @@ static int webm_dash_manifest_cues(AVFormatContext *s, int64_t init_range)
// parse the cues // parse the cues
matroska_parse_cues(matroska); matroska_parse_cues(matroska);
if (!st->internal->nb_index_entries)
return AVERROR_INVALIDDATA;
// cues start // cues start
av_dict_set_int(&s->streams[0]->metadata, CUES_START, cues_start, 0); av_dict_set_int(&s->streams[0]->metadata, CUES_START, cues_start, 0);
@ -4199,22 +4205,19 @@ static int webm_dash_manifest_cues(AVFormatContext *s, int64_t init_range)
// check if all clusters start with key frames // check if all clusters start with key frames
av_dict_set_int(&s->streams[0]->metadata, CLUSTER_KEYFRAME, webm_clusters_start_with_keyframe(s), 0); av_dict_set_int(&s->streams[0]->metadata, CLUSTER_KEYFRAME, webm_clusters_start_with_keyframe(s), 0);
// store cue point timestamps as a comma separated list for checking subsegment alignment in // Store cue point timestamps as a comma separated list
// the muxer. assumes that each timestamp cannot be more than 20 characters long. // for checking subsegment alignment in the muxer.
buf = av_malloc_array(s->streams[0]->internal->nb_index_entries, 20); av_bprint_init(&bprint, 0, AV_BPRINT_SIZE_UNLIMITED);
if (!buf) return -1; for (int i = 0; i < st->internal->nb_index_entries; i++)
strcpy(buf, ""); av_bprintf(&bprint, "%" PRId64",", st->internal->index_entries[i].timestamp);
for (i = 0; i < s->streams[0]->internal->nb_index_entries; i++) { if (!av_bprint_is_complete(&bprint)) {
int ret = snprintf(buf + end, 20, av_bprint_finalize(&bprint, NULL);
"%" PRId64"%s", s->streams[0]->internal->index_entries[i].timestamp, return AVERROR(ENOMEM);
i != s->streams[0]->internal->nb_index_entries - 1 ? "," : "");
if (ret <= 0 || (ret == 20 && i == s->streams[0]->internal->nb_index_entries - 1)) {
av_log(s, AV_LOG_ERROR, "timestamp too long.\n");
av_free(buf);
return AVERROR_INVALIDDATA;
}
end += ret;
} }
// Remove the trailing ','
bprint.str[--bprint.len] = '\0';
if ((ret = av_bprint_finalize(&bprint, &buf)) < 0)
return ret;
av_dict_set(&s->streams[0]->metadata, CUE_TIMESTAMPS, av_dict_set(&s->streams[0]->metadata, CUE_TIMESTAMPS,
buf, AV_DICT_DONT_STRDUP_VAL); buf, AV_DICT_DONT_STRDUP_VAL);