On 07/08/14 18:15, Anton Khirnov wrote:
> 
> 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?

Let me rework it to not use goto fail; at all.

> Failing on sufficiently strict error checking would be appropriate i think.

It requires propagating over 3 calls I plan to do that for master (and 10).

lu
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to