Possible overflow in rwpng_create_row_pointers

[epozuelo]

Hi,

Looking at CVE-2016-5735, I noticed this:

static png_bytepp rwpng_create_row_pointers(png_infop info_ptr, png_structp png_ptr, unsigned char *base, unsigned int height, png_size_t rowbytes)
{
    if (!rowbytes) {
        rowbytes = png_get_rowbytes(png_ptr, info_ptr);
    }

    png_bytepp row_pointers = malloc(height * sizeof(row_pointers[0]));
    if (!row_pointers) return NULL;
    for(size_t row = 0; row < height; row++) {
        row_pointers[row] = base + row * rowbytes;
    }
    return row_pointers;
}

If the parsed file has a huge height, then it would be possible to overflow height * sizeof(row_pointers[0]), wouldn't it? From a quick search, it seems like the width and height limits are 2^31-1, and with such a height, that would overflow on 32 bit systems when multiplying by sizeof(row_pointers[0]), i.e. sizeof(unsigned char *).

[kornelski]

There is a size check before calling rwpng_create_row_pointers

https://github.com/pornel/pngquant/blob/master/rwpng.c#L310

[epozuelo]

IIUC, that check is different. That check is for height * width, or actually for height * row_width (because row_width can be higher than width). However this malloc is doing height * sizeof(pointer).

Now imagine an image that is incredibly high and narrow. e.g. 2^31 px high, and 1px wide. Then if for some reason row_width == 1, the first check will be false. However (height * sizeof(row_pointers[0])) will overflow.

Maybe I'm missing something, or maybe this scenario is not possible because it's not possible to build such an image, or because row_width will be higher in that case. However a quick read of the png spec suggested that this height is possible, so perhaps a check should be added.