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

[Michael Niedermayer]

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

but iam fine with any solution that works

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle
-------------- 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/cf5d1320/attachment.asc>