Avoid overflow in calculateNumTiles when size=MAX_INT

[cary-ilm]

Addresses https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=25297

Signed-off-by: Cary Phillips [email protected]

[peterhillman]

LGTM

[lgritz]

Does this really need to avoid this case and throw an exception, just because an intermediate sum might overflow as an int? How about

int64_t l = levelsize(...);
numTiles[i] = static_cast<int>((l + size - 1) / size);

[peterhillman]

Does this really need to avoid this case and throw an exception, just because an intermediate sum might overflow as an int?
Using 64 bit computation would prevent an overflow, but 'size' here is the width or height of a tile. Why would anyone want to write an image that's 4gigapixel wide and only has one tile? There's also a limit to INT_MAX bytes per tile, which would be reached well before these width/height limits.

I think bailing early is sensible, to help protect against any other issues that might be uncovered handling very large files.

[lgritz]

size is an int. The resolution of the MIPmap level, L is an int. The number of tiles will be an int, since it is, mathematically, ceil(L/size), and thus smaller than L.

The current code oddly disallows the level resolution to be close to intmax, not because it's a real limit, but simply because some values of an intermediate calculation of the expression (L+size-1)/size will cause an overflow in the implied temporary that will hold L+size (because both operands are int, so will be the temporary).

By declaring L as an int64_t, that will make the temporary wide enough to do the (L+size-1)/size calculation correctly, for any valid int value of L.

[peterhillman]

Ah yes, so you can overflow with an image which is only just under INT_MAX wide, even with plausible sized tiles.

Is it correct that the code wouldn't overflow for any mipmap level in any rounding mode if (width of dataWindow)+(tile width) and (height of dataWindow)+(tile height) are both less than INT_MAX-1? That seems like a sane limit to have anyway, since both individually have to be less than INT_MAX.

My point was really that these are insane situations anyway, since the practical limit to image dimensions is significantly less than INT_MAX. All these tests and workarounds do is to protect against 'non-legit' EXR files.

I suppose switching everything to 64 bit would allow the test and the exception to be removed, so would be more efficient, and might be a better way to go.

[meshula]

Good thought, it makes sense to switch to i64 for those reasons.