-
Notifications
You must be signed in to change notification settings - Fork 617
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Avoid overflow in calculateNumTiles when size=MAX_INT #825
Avoid overflow in calculateNumTiles when size=MAX_INT #825
Conversation
Signed-off-by: Cary Phillips <cary@ilm.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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);
I think bailing early is sensible, to help protect against any other issues that might be uncovered handling very large files. |
size is an int. The resolution of the MIPmap level, 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 By declaring L as an int64_t, that will make the temporary wide enough to do the |
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. |
Signed-off-by: Cary Phillips <cary@ilm.com>
Good thought, it makes sense to switch to i64 for those reasons. |
…les when size=MAX_INT Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>
…tests (#875) * ignore unused bits in B44 mode detection Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * apply #832: use unsigned values in shift to prevent undefined behavior Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * apply #818: compute Huf codelengths using 64 bit to prevent shift overflow Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * apply #817: double-check unpackedBuffer created in DWA uncompress Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * apply #820: suppress sanitizer warnings when writing invalid enums Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * apply #825: Avoid overflow in calculateNumTiles when size=MAX_INT Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * apply #826: restrict maximum tile size to INT_MAX byte limit Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * apply #827: lighter weight reading of Luma-only images via RgbaInputFile Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * refactor channel filling in InputFile API with tiled source Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * handle edge-case of empty framebuffer Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * apply #829: fix buffer overflow check in PIZ decompression Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * Use Int64 in dataWindowForTile to prevent integer overflow (#831) * Use Int64 in dataWindowForTile to prevent integer overflow Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * use signed 64 bit instead for dataWindow calculation Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> Co-authored-by: Cary Phillips <cary@ilm.com> Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * prevent overflow in hufUncompress if nBits is large (#836) Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * add sanity check for reading multipart files with no parts (#840) Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * reduce B44 _tmpBufferSize (was allocating two bytes per byte) (#843) Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * check for valid Huf code lengths (#849) * check for valid Huf code lengths * test non-fast huf decoder in testHuf Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * check 1 part files with 'nonimage' bit have type attribute (#860) Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> Co-authored-by: Cary Phillips <cary@ilm.com> Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * Fix overflow computing deeptile sample table size (#861) Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * re-order shift/compare in FastHuf to prevent undefined shift overflow warning (#819) Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> Co-authored-by: Cary Phillips <cary@ilm.com> Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * more elegant exception handling in exrmaketiled (#841) Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * check EXRAllocAligned succeeded to allocate ScanlineInputFile lineBuffers (#844) Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * test channels are DCT compressed before DWA decompression (#845) Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * Merge ABI-compatible changes from #842 Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> Co-authored-by: Cary Phillips <cary@ilm.com>
…eFoundation#825) * Avoid overflow in calculateNumTiles when size=MAX_INT Signed-off-by: Cary Phillips <cary@ilm.com> * Compute level size with 64 bits to avoid overflow Signed-off-by: Cary Phillips <cary@ilm.com>
…eFoundation#825) * Avoid overflow in calculateNumTiles when size=MAX_INT Signed-off-by: Cary Phillips <cary@ilm.com> * Compute level size with 64 bits to avoid overflow Signed-off-by: Cary Phillips <cary@ilm.com>
* double-check unpackedBuffer created in DWA uncompress Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> Signed-off-by: Cary Phillips <cary@ilm.com> * compute Huf codelengths using 64 bit to prevent shift overflow Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> Signed-off-by: Cary Phillips <cary@ilm.com> * Avoid overflow in calculateNumTiles when size=MAX_INT (#825) * Avoid overflow in calculateNumTiles when size=MAX_INT Signed-off-by: Cary Phillips <cary@ilm.com> * Compute level size with 64 bits to avoid overflow Signed-off-by: Cary Phillips <cary@ilm.com> * More efficient handling of filled channels reading tiles with scanline API (#830) * refactor channel filling in InputFile API with tiled source Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * handle edge-case of empty framebuffer Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> Signed-off-by: Cary Phillips <cary@ilm.com> * fix undefined behavior: ignore unused bits in B44 mode detection (#832) Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> Signed-off-by: Cary Phillips <cary@ilm.com> * Fix overflow computing deeptile sample table size (#861) Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> Signed-off-by: Cary Phillips <cary@ilm.com> * sanity check ScanlineInput bytesPerLine instead of lineOffset size (#863) Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> Co-authored-by: Cary Phillips <cary@ilm.com> Signed-off-by: Cary Phillips <cary@ilm.com> * Release notes for v2.4.3 Signed-off-by: Cary Phillips <cary@ilm.com> * Bump version for v2.4.3 Signed-off-by: Cary Phillips <cary@ilm.com> * reduce size limit for scanline files; prevent large chunkoffset allocations (#824) * reduce size limit for scanline files; protect against large chunkoffset allocations Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * bugfix for memory limit changes Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * rearrange chunkoffset test to protect bytesperline table too Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> * remove extraneous function declaration; tidy comments Signed-off-by: Peter Hillman <peterh@wetafx.co.nz> Signed-off-by: Cary Phillips <cary@ilm.com> * Change v2.4.3 release date to May 17, and clean up urls Signed-off-by: Cary Phillips <cary@ilm.com> Co-authored-by: Peter Hillman <peterh@wetafx.co.nz>
Addresses https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=25297
Signed-off-by: Cary Phillips cary@ilm.com