[FFmpeg-devel] [PATCH 01/11] libavformat/mxfdec.c: klv_read_packet: Properly check klv_decode_ber_length return value.

[Tomas Härdin]

On Thu, 2015-10-22 at 19:25 +0200, Alexis Ballier wrote:
> On Wed, 21 Oct 2015 23:31:29 +0200
> Tomas Härdin <tomas.hardin at codemill.se> wrote:
> 
> > On Wed, 2015-10-21 at 18:00 +0200, Alexis Ballier wrote:
> > > klv_decode_ber_length cannot return -1, but can return
> > > AVERROR_INVALIDDATA. Store its return value in a signed integer
> > > (instead of unsigned KLVPacket.length) and forward the error
> > > appropriately. --- libavformat/mxfdec.c | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > > index 00d420b..94a953b 100644
> > > --- a/libavformat/mxfdec.c
> > > +++ b/libavformat/mxfdec.c
> > > @@ -366,13 +366,15 @@ static int mxf_read_sync(AVIOContext *pb,
> > > const uint8_t *key, unsigned size) 
> > >  static int klv_read_packet(KLVPacket *klv, AVIOContext *pb)
> > >  {
> > > +    int64_t len;
> > >      if (!mxf_read_sync(pb, mxf_klv_key, 4))
> > >          return AVERROR_INVALIDDATA;
> > >      klv->offset = avio_tell(pb) - 4;
> > >      memcpy(klv->key, mxf_klv_key, 4);
> > >      avio_read(pb, klv->key + 4, 12);
> > > -    klv->length = klv_decode_ber_length(pb);
> > > -    return klv->length == -1 ? -1 : 0;
> > > +    len = klv_decode_ber_length(pb);
> > > +    klv->length = FFMAX(len, 0);
> > > +    return FFMIN(len, 0);
> > >  }  
> > 
> > Can't klv_read_packet() return int64_t instead?
> 
> you mean change the return type and return klv->length ?
> that might work, but note that what I'm trying to fix is
> klv_read_packet setting length to (unsigned)AVERROR_INVALIDDATA, i.e.,
> something close to 2^64, and returning 0.
> I don't see any problem by doing that, so it is as you prefer.

Oh, of course - don't let length be some bogus value :)
Actually, maybe it doesn't matter so much. Hm

/Tomas