[FFmpeg-devel] [PATCH] nutdec: check maxpos in read_sm_data before reading count

[Michael Niedermayer]

On Sat, Jun 27, 2015 at 05:53:26PM +0200, Andreas Cadhalpun wrote:
> On 27.06.2015 02:31, Michael Niedermayer wrote:
> > On Fri, Jun 26, 2015 at 07:28:36PM +0200, Andreas Cadhalpun wrote:
> >> On 26.06.2015 01:36, Michael Niedermayer wrote:
> >>> On Thu, Jun 25, 2015 at 11:46:41PM +0200, Andreas Cadhalpun wrote:
> >>>> Otherwise sm_size can be larger than size, which results in a negative
> >>>> packet size.
> >>>>
> >>>> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> >>>> ---
> >>>>  libavformat/nutdec.c | 7 ++++++-
> >>>>  1 file changed, 6 insertions(+), 1 deletion(-)
> >>>
> >>>
> >>>
> >>>>
> >>>> diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
> >>>> index 13fb399..43bd27b 100644
> >>>> --- a/libavformat/nutdec.c
> >>>> +++ b/libavformat/nutdec.c
> >>>> @@ -888,7 +888,7 @@ fail:
> >>>>  
> >>>>  static int read_sm_data(AVFormatContext *s, AVIOContext *bc, AVPacket *pkt, int is_meta, int64_t maxpos)
> >>>>  {
> >>>> -    int count = ffio_read_varlen(bc);
> >>>> +    int count;
> >>>>      int skip_start = 0;
> >>>>      int skip_end = 0;
> >>>>      int channels = 0;
> >>>> @@ -898,6 +898,11 @@ static int read_sm_data(AVFormatContext *s, AVIOContext *bc, AVPacket *pkt, int
> >>>>      int height = 0;
> >>>>      int i, ret;
> >>>>  
> >>>> +    if (avio_tell(bc) >= maxpos)
> >>>> +        return AVERROR_INVALIDDATA;
> >>>> +
> >>>> +    count = ffio_read_varlen(bc);
> >>>
> >>> ffio_read_varlen() could move the position beyond maxpos yet return
> >>> 0 so the loop with teh checks inside is skiped
> >>
> >> That is exactly the problem, because then sm_size can be larger than size.
> >> An alternative would be to directly check for that, like in attached patch.
> > 
> > wouldnt checking after the loop im read_sm_data() before returning
> > success be more robust ?
> > It would exit sooner if the problem occurs in the first call
> > and avoid potential integer overflows
> 
> OK, new patch attached.
> 
> > but iam fine with any solution that works
> 
> Me too.
> 
> Best regards,
> Andreas
> 

>  nutdec.c |    3 +++
>  1 file changed, 3 insertions(+)
> 4e07b069348ca9b9d65b7850291448201c4d81f6  0001-nutdec-check-maxpos-in-read_sm_data-before-returning.patch
> From 4e10305531d162fff2a7daac49cc046c771909a9 Mon Sep 17 00:00:00 2001
> From: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> Date: Sat, 27 Jun 2015 17:50:56 +0200
> Subject: [PATCH] nutdec: check maxpos in read_sm_data before returning success

LGTM

thanks

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

There will always be a question for which you do not know the correct answer.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150627/c93cdce5/attachment.asc>