Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.Sign up
DOS bug while reading attributes in Header::readfrom #248
I use ImageMagick to convert exr image, and it report allocate memory failure. After took a look, ImageMagick calls ImfOpenInputFile to open exr file, this function only takes filename as argument, so I think it is a openexr bug.
You can get the POC file from https://github.com/noirfate/test/blob/master/test.exr and
In my debugging, the allocation size is controlled by attacker, so it not only can cause allocation failure and cause huge memory consumption as well.
In the POC file, 0x159~0x15c is the size
And the debug info :
I did a little bit of digging and examined this bug/security issue more.
The problem is that size attribute, that is read in can have an arbitrarily big value and library than allocates so much memory that it will eighter drain system resources or crash the program altogether.
size is read at:
and later used for new allocation:
However (if I'm not mistaken), there is no way to know if given size is right or wrong and the attribute has no theoretical limit. The library does correctly throw an exception (
The real problem is in ImageMagick, which does not expect that exception can be thrown in openexr library and therefore dies together with the raised exception.
Please correct me if I'm wrong because I don't know the openexr that much and maybe my assumptions about the size attribute are wrong. However, if it really cannot be determined, then there is in my opinion no way to handle this situation better way.
I've pushed an update to make an explicit check on the size attribute in this case. This will cause an "Invalid size field in header attribute" exception to be thrown, rather than a "std::badalloc", which should be more informative. It will also prevent address sanitizers treating this as an error.
I believe ImageMagick does handle OpenEXR exceptions correctly as long as an address sanitizer hasn't aborted it first.