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

[Tomas Härdin]

ons 2018-06-13 klockan 20:27 +0200 skrev 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.

I'm pretty sure there's a check for pb==NULL further up in lavf already

/Tomas