[FFmpeg-devel] [PATCH 01/12] avformat/mxfdec: store next_klv in KLVPacket

[Marton Balint]

On Wed, 13 Jun 2018, Tomas Härdin wrote:

> sön 2018-06-10 klockan 12:36 +0200 skrev Marton Balint:
>> > Signed-off-by: Marton Balint <cus at passwd.hu>
>> ---
>>  libavformat/mxf.h    |  1 +
>>  libavformat/mxfdec.c | 13 ++++++++-----
>>  2 files changed, 9 insertions(+), 5 deletions(-)
>> 
>> diff --git a/libavformat/mxf.h b/libavformat/mxf.h
>> index 19f8d8a9f5..93bc2cd075 100644
>> --- a/libavformat/mxf.h
>> +++ b/libavformat/mxf.h
>> @@ -62,6 +62,7 @@ typedef struct KLVPacket {
>>      UID key;
>>      int64_t offset;
>>      uint64_t length;
>> +    int64_t next_klv;
>>  } KLVPacket;
>>  
>>  typedef struct MXFCodecUL {
>> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
>> index b3d3e237c0..a5c5fb3b8a 100644
>> --- a/libavformat/mxfdec.c
>> +++ b/libavformat/mxfdec.c
>> @@ -392,7 +392,7 @@ static int mxf_read_sync(AVIOContext *pb, const uint8_t *key, unsigned size)
>>  
>>  static int klv_read_packet(KLVPacket *klv, AVIOContext *pb)
>>  {
>> -    int64_t length;
>> +    int64_t length, pos;
>>      if (!mxf_read_sync(pb, mxf_klv_key, 4))
>>          return AVERROR_INVALIDDATA;
>>      klv->offset = avio_tell(pb) - 4;
>> @@ -402,6 +402,10 @@ static int klv_read_packet(KLVPacket *klv, AVIOContext *pb)
>>      if (length < 0)
>>          return length;
>>      klv->length = length;
>> +    pos = avio_tell(pb);
>> +    if (pos > INT64_MAX - length)
>> +        return AVERROR_INVALIDDATA;
>
> I wonder, can pos be negative? That is, can avio_tell() fail? Else it
> looks OK

Although it is not documented behaviour, but it can't be negative in the 
current implementation (if pb is not NULL). I can add a check if that is 
preferred.

Regards,
Marton