On Thu, 7 Aug 2014 17:19:32 +0200, Luca Barbato <[email protected]> wrote:
> Prevent out of array write.
>
> Similar to what Michael Niedermayer did to address the same issue.
>
> Bug-Id: CVE-2014-2263
> CC: [email protected]
> ---
>
> It reports the error while trying to write the pmt, but doesn't stop
> as Kieran suggested.
>
> For the master and 10 I'd provide one that optionally exists
> at write_header time.
>
> As long we do not support multiple services sdt (and pat) are fine.
>
> libavformat/mpegtsenc.c | 25 ++++++++++++++++++++++++-
> 1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
> index 86fc631..28ea1bd 100644
> --- a/libavformat/mpegtsenc.c
> +++ b/libavformat/mpegtsenc.c
> @@ -254,7 +254,7 @@ static void mpegts_write_pmt(AVFormatContext *s,
> MpegTSService *service)
> {
> MpegTSWrite *ts = s->priv_data;
> uint8_t data[SECTION_LENGTH], *q, *desc_length_ptr,
> *program_info_length_ptr;
> - int val, stream_type, i;
> + int val, stream_type, i, err = 0;
>
> q = data;
> put16(&q, 0xe000 | service->pcr_pid);
> @@ -272,6 +272,11 @@ static void mpegts_write_pmt(AVFormatContext *s,
> MpegTSService *service)
> AVStream *st = s->streams[i];
> MpegTSWriteStream *ts_st = st->priv_data;
> AVDictionaryEntry *lang = av_dict_get(st->metadata, "language",
> NULL,0);
> +
> + if (q - data > SECTION_LENGTH - 3 - 2 - 6) {
> + err = 1;
> + goto fail;
> + }
> switch(st->codec->codec_id) {
> case AV_CODEC_ID_MPEG1VIDEO:
> case AV_CODEC_ID_MPEG2VIDEO:
> @@ -321,6 +326,10 @@ static void mpegts_write_pmt(AVFormatContext *s,
> MpegTSService *service)
> *len_ptr = 0;
>
> for (p = lang->value; next && *len_ptr < 255 / 4 * 4; p =
> next + 1) {
> + if (q - data > SECTION_LENGTH - 4) {
> + err = 1;
> + goto fail;
> + }
Don't you end up with invalid data when stopping here or below?
> next = strchr(p, ',');
> if (strlen(p) != 3 && (!next || next != p + 3))
> continue; /* not a 3-letter code */
> @@ -355,6 +364,11 @@ static void mpegts_write_pmt(AVFormatContext *s,
> MpegTSService *service)
> *q++ = language[1];
> *q++ = language[2];
> *q++ = 0x10; /* normal subtitles (0x20 = if hearing pb) */
> +
> + if (q - data > SECTION_LENGTH - 4) {
> + err = 1;
> + goto fail;
> + }
> if(st->codec->extradata_size == 4) {
> memcpy(q, st->codec->extradata, 4);
> q += 4;
> @@ -380,6 +394,15 @@ static void mpegts_write_pmt(AVFormatContext *s,
> MpegTSService *service)
> desc_length_ptr[0] = val >> 8;
> desc_length_ptr[1] = val;
> }
> +
> +fail:
> + if (err)
> + av_log(s, AV_LOG_ERROR,
> + "The pmt section cannot fit the stream %d and following.\n"
> + "Try reducing the number of languages in the audio streams "
> + "or the total number of streams.\n",
> + i);
Failing on sufficiently strict error checking would be appropriate i think.
--
Anton Khirnov
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
