On 19.12.2015 13:58, Michael Niedermayer wrote: > 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
OK, updated patch attached. > you could alternatively change value_len to uint64_t which might > simplify the check It wouldn't help, because either avio_tell(bc) + value_len can be too large or maxpos - avio_tell(bc) negative, which would be treated as a large unsigned value, if value_len is unsigned. Best regards, Andreas
>From 98fc98ce850d4d7fce30ee653dd48c063f0eaae6 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. Also fix potential overflow of avio_tell(bc) + value_len. 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 ef08ad9..4df6a57 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 || value_len >= maxpos - avio_tell(bc)) return AVERROR_INVALIDDATA; if (!strcmp(name, "Palette")) { dst = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, value_len); -- 2.6.2
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel