guard against integer overflow in width * height pixel count

[sayedkaif27]

Description

Guard against integer overflow when computing the pixel count in avifRGBImageComputeGainMap.

On 32-bit platforms (size_t = 32 bits), the expression (size_t)width * height can wrap
silently when both uint32_t operands are large, causing the subsequent
numPixels > SIZE_MAX / sizeof(float) check to operate on an already-corrupted value and
pass erroneously — leading to a heap buffer overflow during gain map computation.

Fix: add the standard unsigned overflow pre-check before the multiplication:

if (width != 0 && height > SIZE_MAX / width) {
    res = AVIF_RESULT_INVALID_ARGUMENT;
    goto cleanup;
}

[vrabaud]

Replacing the size_t arithmetic with uint64_t is easier no? There is another (size_t)width in that file (the only other one in the code base).

[sayedkaif27]

I updated the patch to perform the multiplication in uint64_t and added the same change to the other (size_t)width * height

[wantehchang]

Thanks for analyzing the libavif code. The code in question relies on the fact that if a pixel buffer has been allocated successfully, we can multiply the width or the rowBytes (also known as the stride) of the buffer by the height of the buffer using the size_t type.

In the code modified in this pull request, the pixel buffer is an input parameter of the functions. Since the functions did not allocate the pixel buffer, this becomes an API contract that the pixel buffer and its width, height, and rowBytes are all valid.

[wantehchang]

The original code is correct. Please verify the following analysis.

width and height are defined as follows:

    const uint32_t width = baseRgbImage->width;
    const uint32_t height = baseRgbImage->height;

The original code relies on the API contract that baseRgbImage->width and baseRgbImage->height are the width and height of the baseRgbImage->pixels buffer. It is safe to multiply baseRgbImage->rowBytes by baseRgbImage->height using the size_t type. Since baseRgbImage->width <= baseRgbImage->rowBytes, it is also safe to multiply baseRgbImage->width by baseRgbImage->height using the size_t type.

[wantehchang]

The original code is correct. Please check the following analysis.

width and height are defined as follows:

    const uint32_t width = baseImage->width;
    const uint32_t height = baseImage->height;

The original code relies on the API contract that baseImage->width and baseImage->height are the width and height of the baseImage->pixels buffer. It is safe to multiply baseImage->rowBytes by baseImage->height using the size_t type (done at line 119 and inside the avifGetRGBAPixel() call at line 147). Since baseImage->width <= baseImage->rowBytes, it is also safe to multiply baseImage->width by baseImage->height using the size_t type.

[sayedkaif27]

Thanks for the clarification. I’ve updated the code back to size_t.