CVE-2017-6960 in apng2gif

[Hugo Lefeuvre <hle@debian.org>]

Hi,

I've had a look at CVE-2017-6960 and tried to write a patch fixing it
but I'm not 100% sure that my solution is the "right" one.

** Short summary of the issue

A segmentation fault occurs at line 627, in LoadAPNG:

    memset(pOut1, 0, outimg1);

memory allocation for pOut1 is realized at line 615:

    pOut1 = (unsigned char  *)malloc((frames+1)*outimg1*4);

and gdb prints following values:

 * outimg1 = h*outrow = 1090585600
 * h = the h block of the apng image = 524320
 * outrow = the w block of the apng image 2080
 * frames = 1
 * (frames+1)*outimg1*4 = 134750208

So there is an integer overflow when executing (frames+1)*outimg1*4 since

(frames+1)*outimg1*4 should be 1090585600*4*2 = 8724684800 (> UINT_MAX)

but in fact it is only 134750208 < outimg1.

This issue occurs at several places in the source code, e.g. at line 613

    outimg2 = h*outrow2;

or at line 711...

    pOut1   = (unsigned char  *)malloc((frames+1)*outimg1*4);

Images with width and height like this one are perfectly fine according to the
APNG specification, which doesn't set any other limit than w<=UINT_MAX and h<=UINT_MAX
(of course, the test image is just a broken, crafted image, but I'm
speaking in a general sense).

However, I don't know how this kind of big images should be supported, because anyway
they would be way too big to fit in the ram.

** My patch

I propose to add an integer overflow check before the memory allocation so we avoid
the crash and notify the user when passed image is too big to be processed.

Something like

    if( w > UINT_MAX/(4*(frames+1)*h) || h > UINT_MAX/(4*(frames+1)*w) ) {
        printf("Error: Image size unsupported\n");
        break;
    }

since

     w*h*4*(frames+1) > MAXINT
<=>               w*h > MAXINT/(4*(frames+1))    // I except frames+1 to say positive
<=>  w > MAXINT/(4*(frames+1)*h)
     or
     h > MAXINT/(4*(frames+1)*w)

Tested in practice, it works.

Does anybody have an alternative, maybe more elegant solution idea or integer overflow check ?

Cheers,
 Hugo

-- 
             Hugo Lefeuvre (hle)    |    www.owl.eu.com
4096/ ACB7 B67F 197F 9B32 1533 431C AC90 AC3E C524 065E

Attachment: signature.asc
Description: PGP signature