diff mbox series

[FFmpeg-devel,v3,2/4] avformat/apngdec: Fix size/overflow checks

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

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
apng data consists of parts containing a small header (including a
four-byte size field) and a data part; the size field does not account
for everything and is actually twelve bytes short of the actual size. In
order to make sure that the size fits into an int, the size field is
checked for being > INT_MAX; yet this does not account for the + 12 and
upon conversion to int (which happens when calling append_extradata()),
the size parameter can still wrap around. In this case the currently
used check would lead to undefined signed integer overflow.

Furthermore, append_extradata() appends the new data to the already
existing extradata and therefore needs to make sure that the combined
size of new and old data as well as padding fits into an int. The check
used for this is "if (old_size > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE -
new_size)". If new_size is > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE
the right side becomes negative if the types are signed (as they are
now); yet changing this to "if (new_size > INT_MAX -
AV_INPUT_BUFFER_PADDING_SIZE - old_size)" is better as this also works
for unsigned types (where it is of course presumed that INT_MAX is
replaced by the corresponding maximum for the new type).

Both of these issues have been fixed.

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

Patch

diff --git a/libavformat/apngdec.c b/libavformat/apngdec.c
index 23d7e15393..d8d0de190f 100644
--- a/libavformat/apngdec.c
+++ b/libavformat/apngdec.c
@@ -127,7 +127,7 @@  static int append_extradata(AVCodecParameters *par, AVIOContext *pb, int len)
     int new_size, ret;
     uint8_t *new_extradata;
 
-    if (previous_size > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE - len)
+    if (len > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE - previous_size)
         return AVERROR_INVALIDDATA;
 
     new_size = previous_size + len;
@@ -208,7 +208,7 @@  static int apng_read_header(AVFormatContext *s)
             goto fail;
 
         len = avio_rb32(pb);
-        if (len > 0x7fffffff) {
+        if (len > INT_MAX - 12) {
             ret = AVERROR_INVALIDDATA;
             goto fail;
         }