| Message ID | c6b9d2c2-f093-2584-d763-20944641683e@googlemail.com |
|---|---|
| State | Accepted |
| Headers | show |
On Wed, Nov 09, 2016 at 08:56:00PM +0100, Andreas Cadhalpun wrote: > On 09.11.2016 02:31, Michael Niedermayer wrote: > > On Tue, Nov 08, 2016 at 11:36:58PM +0100, Andreas Cadhalpun wrote: > >> It can read less than the requested amount, in which case buf contains > >> uninitialized data, causing problems like segmentation faults later on. > >> > >> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> > >> --- > >> libavformat/icodec.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/libavformat/icodec.c b/libavformat/icodec.c > >> index 8019a35..aad1416 100644 > >> --- a/libavformat/icodec.c > >> +++ b/libavformat/icodec.c > >> @@ -174,8 +174,8 @@ static int read_packet(AVFormatContext *s, AVPacket *pkt) > >> bytestream_put_le16(&buf, 0); > >> bytestream_put_le32(&buf, 0); > >> > >> - if ((ret = avio_read(pb, buf, image->size)) < 0) > >> - return ret; > >> + if ((ret = avio_read(pb, buf, image->size)) != image->size) > >> + return ret < 0 ? ret : AVERROR_INVALIDDATA; > > > > is anything checking size to be positive ? > > if not it could be matching an error code i think > > I've added a check to make sure that size is positive. New patch attached. > > Best regards, > Andreas > > icodec.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > 04c12ac83fea7b911f4050c547b77d1c48e9228b 0001-icodec-correctly-check-avio_read-return-value.patch > From 31f78a6d2906de4ee69e472aced5fef3af709b9a Mon Sep 17 00:00:00 2001 > From: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> > Date: Tue, 8 Nov 2016 23:29:28 +0100 > Subject: [PATCH] icodec: correctly check avio_read return value > > It can read less than the requested amount, in which case buf contains > uninitialized data, causing problems like segmentation faults later on. > > Also make sure that image->size is positive, so that it can't match a > negative error code. > > Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> > --- > libavformat/icodec.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/libavformat/icodec.c b/libavformat/icodec.c > index 8019a35..c273bf6 100644 > --- a/libavformat/icodec.c > +++ b/libavformat/icodec.c should be ok thx [...]
On 10.11.2016 21:14, Michael Niedermayer wrote: > On Wed, Nov 09, 2016 at 08:56:00PM +0100, Andreas Cadhalpun wrote: >> icodec.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> 04c12ac83fea7b911f4050c547b77d1c48e9228b 0001-icodec-correctly-check-avio_read-return-value.patch >> From 31f78a6d2906de4ee69e472aced5fef3af709b9a Mon Sep 17 00:00:00 2001 >> From: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> >> Date: Tue, 8 Nov 2016 23:29:28 +0100 >> Subject: [PATCH] icodec: correctly check avio_read return value >> >> It can read less than the requested amount, in which case buf contains >> uninitialized data, causing problems like segmentation faults later on. >> >> Also make sure that image->size is positive, so that it can't match a >> negative error code. >> >> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> >> --- >> libavformat/icodec.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/libavformat/icodec.c b/libavformat/icodec.c >> index 8019a35..c273bf6 100644 >> --- a/libavformat/icodec.c >> +++ b/libavformat/icodec.c > > should be ok Pushed. Best regards, Andreas
From 31f78a6d2906de4ee69e472aced5fef3af709b9a Mon Sep 17 00:00:00 2001 From: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> Date: Tue, 8 Nov 2016 23:29:28 +0100 Subject: [PATCH] icodec: correctly check avio_read return value It can read less than the requested amount, in which case buf contains uninitialized data, causing problems like segmentation faults later on. Also make sure that image->size is positive, so that it can't match a negative error code. Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> --- libavformat/icodec.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libavformat/icodec.c b/libavformat/icodec.c index 8019a35..c273bf6 100644 --- a/libavformat/icodec.c +++ b/libavformat/icodec.c @@ -109,6 +109,10 @@ static int read_header(AVFormatContext *s) avio_skip(pb, 5); ico->images[i].size = avio_rl32(pb); + if (ico->images[i].size <= 0) { + av_log(s, AV_LOG_ERROR, "Invalid image size %d\n", ico->images[i].size); + return AVERROR_INVALIDDATA; + } ico->images[i].offset = avio_rl32(pb); if (avio_seek(pb, ico->images[i].offset, SEEK_SET) < 0) @@ -174,8 +178,8 @@ static int read_packet(AVFormatContext *s, AVPacket *pkt) bytestream_put_le16(&buf, 0); bytestream_put_le32(&buf, 0); - if ((ret = avio_read(pb, buf, image->size)) < 0) - return ret; + if ((ret = avio_read(pb, buf, image->size)) != image->size) + return ret < 0 ? ret : AVERROR_INVALIDDATA; st->codecpar->bits_per_coded_sample = AV_RL16(buf + 14); -- 2.10.2