diff mbox series

[FFmpeg-devel,v3,1/4] avformat/apngdec: Return error for incomplete header

Message ID 20201031141626.727000-1-andreas.rheinhardt@gmail.com
State Accepted
Commit 2cf1eefae5dba7a7259156f9ff5c62f4e4e2fe0d
Headers show
Series [FFmpeg-devel,v3,1/4] avformat/apngdec: Return error for incomplete header | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished

Commit Message

Andreas Rheinhardt Oct. 31, 2020, 2:16 p.m. UTC
If avio_read() could read anything, it returns the number of bytes read,
even if it could not read as much as the caller desired.
apng_read_header() only checked the return value of its avio_read() calls
for being negative and this meant that it was possible for an incomplete
header to not be detected. The return value of the last successfull call
has been returned instead. This commit changes this.

Fixes: OOM
Fixes: 26608/clusterfuzz-testcase-minimized-ffmpeg_dem_APNG_fuzzer-4839491644424192

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavformat/apngdec.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Michael Niedermayer Nov. 2, 2020, 11:33 p.m. UTC | #1
On Sat, Oct 31, 2020 at 03:16:23PM +0100, Andreas Rheinhardt wrote:
> If avio_read() could read anything, it returns the number of bytes read,
> even if it could not read as much as the caller desired.
> apng_read_header() only checked the return value of its avio_read() calls
> for being negative and this meant that it was possible for an incomplete
> header to not be detected. The return value of the last successfull call
> has been returned instead. This commit changes this.
> 
> Fixes: OOM
> Fixes: 26608/clusterfuzz-testcase-minimized-ffmpeg_dem_APNG_fuzzer-4839491644424192
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavformat/apngdec.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

I would have left the fix for the OOM and the 9 byte read seperate
but the patch LGTM, please apply

thx


[...]
diff mbox series

Patch

diff --git a/libavformat/apngdec.c b/libavformat/apngdec.c
index 0f1d04a365..23d7e15393 100644
--- a/libavformat/apngdec.c
+++ b/libavformat/apngdec.c
@@ -138,7 +138,7 @@  static int append_extradata(AVCodecParameters *par, AVIOContext *pb, int len)
     par->extradata = new_extradata;
     par->extradata_size = new_size;
 
-    if ((ret = avio_read(pb, par->extradata + previous_size, len)) < 0)
+    if ((ret = ffio_read_size(pb, par->extradata + previous_size, len)) < 0)
         return ret;
 
     return previous_size;
@@ -185,10 +185,10 @@  static int apng_read_header(AVFormatContext *s)
     AV_WL32(st->codecpar->extradata+4,  tag);
     AV_WB32(st->codecpar->extradata+8,  st->codecpar->width);
     AV_WB32(st->codecpar->extradata+12, st->codecpar->height);
-    if ((ret = avio_read(pb, st->codecpar->extradata+16, 9)) < 0)
-        goto fail;
+    if ((ret = ffio_read_size(pb, st->codecpar->extradata + 16, 9)) < 0)
+        return ret;
 
-    while (!avio_feof(pb)) {
+    while (1) {
         if (acTL_found && ctx->num_play != 1) {
             int64_t size   = avio_size(pb);
             int64_t offset = avio_tell(pb);