[FFmpeg-devel] [PATCH 1/2] avformat/flvdec: implement support for parsing ModEx data

[Timo Rothenpieler]

On 15/01/2025 08:48, Leo Izen wrote:
> On 1/14/25 7:36 PM, Timo Rothenpieler wrote:
>> ---
>>   libavformat/flv.h    |  5 ++++
>>   libavformat/flvdec.c | 63 ++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 68 insertions(+)
>>
>> diff --git a/libavformat/flv.h b/libavformat/flv.h
>> index 74d3b8de8b..d8f7980f2e 100644
>> --- a/libavformat/flv.h
>> +++ b/libavformat/flv.h
>> @@ -130,6 +130,7 @@ enum {
>>       PacketTypeMetadata              = 4,
>>       PacketTypeMPEG2TSSequenceStart  = 5,
>>       PacketTypeMultitrack            = 6,
>> +    PacketTypeModEx                 = 7,
>>   };
>>   enum {
>> @@ -139,6 +140,10 @@ enum {
>>       AudioPacketTypeMultitrack         = 5,
>>   };
>> +enum {
>> +    PacketModExTypeTimestampOffsetNano = 0,
>> +};
>> +
>>   enum {
>>       AudioChannelOrderUnspecified = 0,
>>       AudioChannelOrderNative      = 1,
>> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
>> index 2f46475d08..b4ea984f58 100644
>> --- a/libavformat/flvdec.c
>> +++ b/libavformat/flvdec.c
>> @@ -1279,6 +1279,57 @@ static int 
>> flv_update_video_color_info(AVFormatContext *s, AVStream *st)
>>       return 0;
>>   }
>> +static int flv_parse_mod_ex_data(AVFormatContext *s, int *pkt_type, 
>> int *size, int64_t *dts)
>> +{
>> +    int ex_type, ret;
>> +    uint8_t *ex_data;
>> +
>> +    uint16_t ex_size = (uint8_t)avio_r8(s->pb) + 1;
>> +    *size -= 1;
>> +
>> +    if (ex_size == 256) {
>> +        ex_size = (uint16_t)avio_rb16(s->pb);
>> +        *size -= 2;
>> +    }
>> +
>> +    ex_data = av_malloc(ex_size);
>> +    if (!ex_data)
>> +        return AVERROR(ENOMEM);
>> +
>> +    ret = avio_read(s->pb, ex_data, ex_size);
>> +    if (ret < 0) {
>> +        av_free(ex_data);
>> +        return ret;
>> +    }
>> +    *size -= ex_size;
> 
> 
> Is it possible here for ex_size >= size in this case? If so, should we 
> return AVERROR_INVALIDDATA?

With a malformed bitstream, that would be possible.
The main parser function has code at the end that will detect a size 
discrepancy and do its best to re-sync the stream.
Though skipping parsing is the bitstream is obviously nonsense like that 
seems like a good idea.

Since the size here is limited to at most 65k bytes, I don't see any 
potential for any kind of DOS/memory exhaustion tricks to be pulled here 
at least.
At worst it'd run into a premature EOF.

Will add a check and send v2 later tonight.