AWARE SYSTEMS
TIFF and LibTiff Mail List Archive

Thread

2017.06.27 20:51 "[Tiff] Excessive memory allocation while chopping strips", by Nicolas RUFF
2017.06.27 22:08 "Re: [Tiff] Excessive memory allocation while chopping strips", by Even Rouault
2017.06.27 22:43 "Re: [Tiff] Excessive memory allocation while chopping strips", by Olivier Paquet
2017.06.27 22:52 "Re: [Tiff] Excessive memory allocation while chopping strips", by Even Rouault
2017.06.28 02:26 "Re: [Tiff] Excessive memory allocation while chopping strips", by Olivier Paquet
2017.06.28 10:09 "Re: [Tiff] Excessive memory allocation while chopping strips", by Even Rouault
2017.06.28 13:49 "Re: [Tiff] Excessive memory allocation while chopping strips", by Olivier Paquet
2017.06.28 14:36 "Re: [Tiff] Excessive memory allocation while chopping strips", by Even Rouault
2017.06.28 15:22 "Re: [Tiff] Excessive memory allocation while chopping strips", by Olivier Paquet
2017.06.28 15:42 "Re: [Tiff] Excessive memory allocation while chopping strips", by Bob Friesenhahn
2017.06.28 19:01 "Re: [Tiff] Excessive memory allocation while chopping strips", by Rob Tillaart
2017.06.28 20:37 "Re: [Tiff] Excessive memory allocation while chopping strips", by Bob Friesenhahn
2017.06.29 05:52 "Re: [Tiff] Excessive memory allocation while chopping strips", by Paavo Helde
2017.06.28 18:08 "Re: [Tiff] Excessive memory allocation while chopping strips", by Paavo Helde

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