On Sat, Dec 19, 2015 at 12:19:44PM +0100, Andreas Cadhalpun wrote:
> On 19.12.2015 01:51, Michael Niedermayer wrote:
> > On Fri, Dec 18, 2015 at 05:22:31PM +0100, Andreas Cadhalpun wrote:
> >> If it is negative, it makes size larger than the size of the packet
> >> buffer, causing invalid writes in avio_read.
> >>
> >> Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
> >> ---
> >> libavformat/nutdec.c | 5 +++++
> >> 1 file changed, 5 insertions(+)
> >>
> >> diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
> >> index 286d1ee..47ae7a7 100644
> >> --- a/libavformat/nutdec.c
> >> +++ b/libavformat/nutdec.c
> >> @@ -1146,6 +1146,11 @@ static int decode_frame(NUTContext *nut, AVPacket
> >> *pkt, int frame_code)
> >> goto fail;
> >> }
> >> sm_size = avio_tell(bc) - pkt->pos;
> >> + if (sm_size < 0) {
> >
> > did sm_size overflow and should be 64bit ?
>
> No.
>
> > did the byte position (avio_tell) move backward ?
>
> Yes.
>
> > (this should not happen)
>
> In that case, the check needs to be moved to read_sm_data.
> Patch doing that is attached.
>
> Best regards,
> Andreas
> > nutdec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > 2f0ac251ee05a8a36ffbaec5a9d5346ac0ef4240 > 0001-nutdec-reject-negative-value_len-in-read_sm_data.patch > From bdca159087d426f5f989656a08464fec967b4bc3 Mon Sep 17 00:00:00 2001 > From: Andreas Cadhalpun <andreas.cadhal...@googlemail.com> > Date: Sat, 19 Dec 2015 12:02:56 +0100 > Subject: [PATCH] nutdec: reject negative value_len in read_sm_data > > If it is negative, it can cause the byte position to move backwards in > avio_skip, which in turn makes sm_size negative and thus size larger > than the size of the packet buffer, causing invalid writes in avio_read. > > Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com> > --- > libavformat/nutdec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c > index 286d1ee..b33b3e2 100644 > --- a/libavformat/nutdec.c > +++ b/libavformat/nutdec.c > @@ -934,7 +934,7 @@ static int read_sm_data(AVFormatContext *s, AVIOContext > *bc, AVPacket *pkt, int > return ret; > } > value_len = ffio_read_varlen(bc); > - if (avio_tell(bc) + value_len >= maxpos) > + if (value_len < 0 || avio_tell(bc) + value_len >= maxpos) > return AVERROR_INVALIDDATA; ok, also while at it please fix avio_tell(bc) + value_len, which i belive can overflow you could alternatively change value_len to uint64_t which might simplify the check [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB It is what and why we do it that matters, not just one of them.
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
