Skip to content
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

enforce limit on area of deep tiles to prevent excessive memory use #939

Merged

Conversation

peterhillman
Copy link
Contributor

This change enforces a hard limit on deep tile sizes, so that xSize*ySize must be less than 2^30, approx 1gigapixel per tile.
This would require more than 2GB of memory to allocate the 'sample count' table. This is not possible on 32 bit architectures, and causes excessive memory allocation on 64 bit architectures.

It is assumed that 'real' files will have much smaller deep tiles (1024x1024 pixels is already quite large), so files that contain tileDescription attributes indicating very large tiles are either corrupt or malicious.

This change is to Deep tiles only. A similar limit is already imposed on regular Tiled images, since the format cannot store compressed tiles with more than 2^31 bytes.

Address:
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=31390
(additionally, a bug in Imf::CheckFile meant that the large tiles were not being detected)

Signed-off-by: Peter Hillman peterh@wetafx.co.nz

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>

if(_data->maxSampleCountTableSize > std::numeric_limits<unsigned int>::max())
{
THROW(IEX_NAMESPACE::ArgExc, "Version " << _data->header.version() << " not supported for deeptiled images in this version of the library");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was this meant to say "tiles requiring over two gigabytes for the sample count table are not supported"?


if(_data->maxSampleCountTableSize > std::numeric_limits<unsigned int>::max())
{
THROW(IEX_NAMESPACE::ArgExc, "Deep tile size exceeds maximum permitted area");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or this message :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, that was embarrassing! Exception message now revised

Signed-off-by: Peter Hillman <peterh@wetafx.co.nz>
@peterhillman peterhillman merged commit 6337740 into AcademySoftwareFoundation:master Feb 26, 2021
@peterhillman peterhillman deleted the deeptile_sizelimit branch February 26, 2021 03:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants