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.cadhal...@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

>From 31f78a6d2906de4ee69e472aced5fef3af709b9a Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun <andreas.cadhal...@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.cadhal...@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

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to