2012.12.12 23:42 "[Tiff] Dealing with unportability of malloc(0)", by Tom Lane

2012.12.13 03:08 "Re: [Tiff] Dealing with unportability of malloc(0)", by Bob Friesenhahn

On Wed, 12 Dec 2012, Tom Lane wrote:

As far as I am aware, the malloc() on most systems returns NULL for a size of zero. We could create a test to confirm this if necessary.

Actually, that's the minority position. A quick check shows that Linux (glibc), Mac OS X, and HP-UX all return a nonnull value for malloc(0) --- that's the only Unix-oid platforms I have handy to check.

It seems that Solaris, FreeBSD, and Microsoft Windows behave similarly. Every day I learn something new. :-)

Regardless, even on the above platforms it is still possible for malloc(0) to return NULL if there is insufficient memory.

The key question for libtiff of course is how often it might be important to cope with zero-length arrays. I'm not sure whether the TIFF spec forbids images of height or width zero, for instance, but libtiff has a problem with them.

I have no idea if libtiff needs to deal with such cases.

How does one create a TIFF file with height and width of zero to test with? Is there ready-made software which can to that? Or do you take a 1x1 TIFF and use a binary editor to make it a 0x0 TIFF?

% gm convert -size 1x1 xc:white zero.tiff
% tiffset -s 256 0 zero.tiff
% tiffset -s 257 0 zero.tiff
TIFFReadDirectory: Cannot handle zero scanline size.
% tiffinfo zero.tiff

TIFFReadDirectory: Cannot handle zero scanline size.

% gm convert -size 1x1 xc:white zero.tiff
% tiffset -s 256 0 zero.tiff
% tiffinfo zero.tiff

TIFFReadDirectory: Cannot handle zero scanline size.

It seems that one can set the image width to zero but not the height. Later tiffinfo refuses to deal with the image.

None of this is terribly hard to fix, but first we need a policy decision about which platform-specific behavior we want to standardize on. I gave my argument above, does anyone want to advocate for the other way? Are there pieces of libtiff where the null-for-zero convention is preferable?

One reason why I like an argument of zero to produce NULL is that often there is a chunk of complex code which computes the size to allocate. In order to assure security, this complex code also needs to detect overflow conditions. Regardless, the decision then needs to be made how to report the overflow condition. A simple approach I have used in the past is to arrange to pass zero to the memory allocation function since there was always already error handling for memory allocation failure. This allows one error reporting path to handle both types of errors.

Yeah, I've noticed that a lot of the size-calculation functions in libtiff use a convention that zero return means overflow. That's fine as long as you're sure empty arrays can never be a legitimate case... but I think that's an assumption that will rise up to bite you eventually.

Zero means means overflow is definitely convenient. Otherwise you would need to arrange to return from the problem code right away or produce both the computed size value and a pass/fail value, with additional conditional code.

The long and the short of it is that we may be up against having to look at every single malloc call site if we want to be safe, whichever way we jump.

Yes, we should have a common convention which is used throught the code. Unfortunately, the various piecemeal security patches have produced a variety of ways to handle sanity checks of calculations. Quite often these patches balloon the code from one or two lines to seven or ten lines.

Integer overflow is pretty challenging to detect and handle reliably. One problem is that some approaches will be optimized away by compilers or have undefined results. Code which is assumed to be working might not be doing anything at all.

http://stackoverflow.com/questions/199333/best-way-to-detect-integer-overflow-in-c-c

http://www.sei.cmu.edu/library/abstracts/reports/10tn008.cfm

More thoughts?

Bob
--
Bob Friesenhahn
bfriesen@simple.dallas.tx.us, http://www.simplesystems.org/users/bfriesen/
GraphicsMagick Maintainer, http://www.GraphicsMagick.org/