-
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 13:49 "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 02:26 "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.27 22:43 "Re: [Tiff] Excessive memory allocation while chopping strips", by Olivier Paquet
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:
...
/*
- Some manufacturers make life difficult by writing
- large amounts of uncompressed data as a single strip.
- This is contrary to the recommendations of the spec.
- The following makes an attempt at breaking such images
- into strips closer to the recommended 8k bytes. A
- side effect, however, is that the RowsPerStrip tag
- value may be changed.
*/
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;
- /*
- * XXX: Check for integer overflow.
- */
- if (nmemb && elem_size && bytes / elem_size == nmemb)
+ /* 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