[FFmpeg-devel] [PATCH 1/4] avformat/apngdec: Check for incomplete reads in append_extradata()

[Andreas Rheinhardt]

Michael Niedermayer:
> On Thu, Oct 29, 2020 at 02:25:49PM +0100, Andreas Rheinhardt wrote:
>> Michael Niedermayer:
>>> Fixes: OOM
>>> Fixes: 26608/clusterfuzz-testcase-minimized-ffmpeg_dem_APNG_fuzzer-4839491644424192
>>>
>>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>>> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
>>> ---
>>>  libavformat/apngdec.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/libavformat/apngdec.c b/libavformat/apngdec.c
>>> index 0f1d04a365..2e79fdd85c 100644
>>> --- a/libavformat/apngdec.c
>>> +++ b/libavformat/apngdec.c
>>> @@ -140,6 +140,8 @@ static int append_extradata(AVCodecParameters *par, AVIOContext *pb, int len)
>>>  
>>>      if ((ret = avio_read(pb, par->extradata + previous_size, len)) < 0)
>>>          return ret;
>>> +    if (ret < len)
>>> +        return AVERROR_INVALIDDATA;
>>>  
>>>      return previous_size;
>>>  }
>>>
>> Reminds me of
>> https://ffmpeg.org/pipermail/ffmpeg-devel/2020-January/255671.html. But
>> how can this fix an OOM scenario? If avio_read() couldn't read
>> everything it should read, then we are at the end of the file and the
>> avio_feof() check will make sure that this is the last iteration of the
>> loop. Or is this a file that is being written to while it is read? (In
>> which case an earlier reading attempt might have failed, but a new one
>> might succeed because there is new data.)
> 
> The OOM occurs when the gigiabyte? sized uninitialized extradata is copied
> and moved around later outside the demuxer
> 

Yeah, I've forgotten: In this case the demuxer does not indicate an
error at all (which is precisely what I wanted to fix).

> If you prefer your patch from january that should achieve the same.
> 
Actually I think one should use ffio_read_size() here (and one should
also adapt ffio_read_size() to forward the error if avio_read() returned
an error, unless said error was AVERROR_EOF).

- Andreas