2017.06.27 20:51 "[Tiff] Excessive memory allocation while chopping strips", by Nicolas RUFF

2017.06.27 20:51 "[Tiff] Excessive memory allocation while chopping strips", by Nicolas RUFF

Hello,

I saw in https://trac.osgeo.org/gdal/changeset/39082 that you try to defend against overly large memory allocations.

I have a pathological test case from fuzzing where header values are:

ImageWidth = 1060
ImageLength = 536871550

It will hit the following piece of code in tif_dirread.c:

...
/*

*/
if ((tif->tif_dir.td_planarconfig==PLANARCONFIG_CONTIG)&&
   (tif->tif_dir.td_nstrips==1)&&
   (tif->tif_dir.td_compression==COMPRESSION_NONE)&&
   ((tif->tif_flags&(TIFF_STRIPCHOP|TIFF_ISTILED))==TIFF_STRIPCHOP))
    {
        if ( !_TIFFFillStriles(tif) || !tif->tif_dir.td_stripbytecount )
            return 0;
        ChopUpSingleUncompressedStrip(tif);
    }
...

ChopUpSingleUncompressedStrip() will compute the required number of strips and reach this point:

...
newcounts = (uint64*) _TIFFCheckMalloc(tif, nstrips, sizeof (uint64),
"for chopped \"StripByteCounts\" array");
...

In the my case, nstrips = 268435775 (and rowsperstrip = 2), which will try to allocate 268435775 * 8 bytes. This is already beyond 2**31, but we can make it even larger and beyond 2**32 by playing with ImageLength.

Rather than playing this cat-and-mouse game of assigning arbitrary limits to every possible tag, I would suggest the following patch in tif_aux.c:

...
#include <math.h>
+#include <limits.h>
...
void*
_TIFFCheckRealloc(TIFF* tif, void* buffer,
 tmsize_t nmemb, tmsize_t elem_size, const char* what)
{
void* cp = NULL;
tmsize_t bytes = nmemb * elem_size;

+ /* TIFF size is limited to 32-bit by design and 31-bit by implementation, so

+  * there should be no reason to ever allocate more than INT_MAX bytes.
+  */+  if ((nmemb <= INT_MAX) && (elem_size <= INT_MAX) && (bytes <=

INT_MAX))
cp = _TIFFrealloc(buffer, bytes);

Note: this patch assumes that tmsize_t is 64 bits, otherwise the check should be done in a different way.

What do you think?

Regards,
- Nicolas RUFF