commit 1826164f90d97b7207247ad268fd2622cd1c6717 Author: drolon Date: Mon Nov 11 05:45:27 2019 +0000 improved TIFF plugin when working with malicious images git-svn-id: https://svn.code.sf.net/p/freeimage/svn@1825 f6e0daa0-2725-47c6-9c0b-5e6e9cdd0720 diff --git a/Source/FreeImage/PluginTIFF.cpp b/Source/FreeImage/PluginTIFF.cpp index f85c2201..a8053196 100644 --- a/Source/FreeImage/PluginTIFF.cpp +++ b/Source/FreeImage/PluginTIFF.cpp @@ -122,9 +122,14 @@ static void ReadThumbnail(FreeImageIO *io, fi_handle handle, void *data, TIFF *t static int s_format_id; typedef struct { + //! FreeImage IO functions FreeImageIO *io; + //! FreeImage handle fi_handle handle; + //! LibTIFF handle TIFF *tif; + //! Count the number of thumbnails already read (used to avoid recursion on loading) + unsigned thumbnailCount; } fi_TIFFIO; // ---------------------------------------------------------- @@ -184,10 +189,8 @@ Open a TIFF file descriptor for reading or writing */ TIFF * TIFFFdOpen(thandle_t handle, const char *name, const char *mode) { - TIFF *tif; - // Open the file; the callback will set everything up - tif = TIFFClientOpen(name, mode, handle, + TIFF *tif = TIFFClientOpen(name, mode, handle, _tiffReadProc, _tiffWriteProc, _tiffSeekProc, _tiffCloseProc, _tiffSizeProc, _tiffMapProc, _tiffUnmapProc); @@ -460,12 +463,10 @@ CreateImageType(BOOL header_only, FREE_IMAGE_TYPE fit, int width, int height, ui } } - else { - - dib = FreeImage_AllocateHeader(header_only, width, height, MIN(bpp, 32), FI_RGBA_RED_MASK, FI_RGBA_GREEN_MASK, FI_RGBA_BLUE_MASK); + else if (bpp <= 32) { + dib = FreeImage_AllocateHeader(header_only, width, height, bpp, FI_RGBA_RED_MASK, FI_RGBA_GREEN_MASK, FI_RGBA_BLUE_MASK); } - } else { // other bitmap types @@ -1067,9 +1068,12 @@ static void * DLL_CALLCONV Open(FreeImageIO *io, fi_handle handle, BOOL read) { // wrapper for TIFF I/O fi_TIFFIO *fio = (fi_TIFFIO*)malloc(sizeof(fi_TIFFIO)); - if(!fio) return NULL; + if (!fio) { + return NULL; + } fio->io = io; fio->handle = handle; + fio->thumbnailCount = 0; if (read) { fio->tif = TIFFFdOpen((thandle_t)fio, "", "r"); @@ -1125,6 +1129,27 @@ check for uncommon bitspersample values (e.g. 10, 12, ...) */ static BOOL IsValidBitsPerSample(uint16 photometric, uint16 bitspersample, uint16 samplesperpixel) { + // get the pixel depth in bits + const uint16 pixel_depth = bitspersample * samplesperpixel; + + // check for a supported pixel depth + switch (pixel_depth) { + case 1: + case 4: + case 8: + case 16: + case 24: + case 32: + case 48: + case 64: + case 96: + case 128: + // OK, go on + break; + default: + // unsupported pixel depth + return FALSE; + } switch(bitspersample) { case 1: @@ -1165,6 +1190,8 @@ IsValidBitsPerSample(uint16 photometric, uint16 bitspersample, uint16 samplesper default: return FALSE; } + + return FALSE; } static TIFFLoadMethod @@ -1254,16 +1281,31 @@ Read embedded thumbnail static void ReadThumbnail(FreeImageIO *io, fi_handle handle, void *data, TIFF *tiff, FIBITMAP *dib) { FIBITMAP* thumbnail = NULL; + + fi_TIFFIO *fio = (fi_TIFFIO*)data; + + /* + Thumbnail loading can cause recursions because of the way + functions TIFFLastDirectory and TIFFSetSubDirectory are working. + We use here a hack to count the number of times the ReadThumbnail function was called. + We only allow one call, check for this + */ + if (fio->thumbnailCount > 0) { + return; + } + else { + // update the thumbnail count (used to avoid recursion) + fio->thumbnailCount++; + } // read exif thumbnail (IFD 1) ... - /* - // this code can cause unwanted recursion causing an overflow, it is thus disabled until we have a better solution - // do we really need to read a thumbnail from the Exif segment ? knowing that TIFF store the thumbnail in the subIFD ... - // toff_t exif_offset = 0; if(TIFFGetField(tiff, TIFFTAG_EXIFIFD, &exif_offset)) { + // this code can cause unwanted recursion causing an overflow, because of the way TIFFLastDirectory work + // => this is checked using + if(!TIFFLastDirectory(tiff)) { // save current position const long tell_pos = io->tell_proc(handle); @@ -1273,15 +1315,15 @@ ReadThumbnail(FreeImageIO *io, fi_handle handle, void *data, TIFF *tiff, FIBITMA int page = 1; int flags = TIFF_DEFAULT; thumbnail = Load(io, handle, page, flags, data); + // store the thumbnail (remember to release it before return) FreeImage_SetThumbnail(dib, thumbnail); - + // restore current position io->seek_proc(handle, tell_pos, SEEK_SET); TIFFSetDirectory(tiff, cur_dir); } } - */ // ... or read the first subIFD @@ -1297,12 +1339,15 @@ ReadThumbnail(FreeImageIO *io, fi_handle handle, void *data, TIFF *tiff, FIBITMA // save current position const long tell_pos = io->tell_proc(handle); const uint16 cur_dir = TIFFCurrentDirectory(tiff); + + // this code can cause unwanted recursion causing an overflow, because of the way TIFFSetSubDirectory work if(TIFFSetSubDirectory(tiff, subIFD_offsets[0])) { // load the thumbnail int page = -1; int flags = TIFF_DEFAULT; thumbnail = Load(io, handle, page, flags, data); + // store the thumbnail (remember to release it before return) FreeImage_SetThumbnail(dib, thumbnail); } @@ -2058,7 +2103,7 @@ Load(FreeImageIO *io, fi_handle handle, int page, int flags, void *data) { } // calculate src line and dst pitch - int dst_pitch = FreeImage_GetPitch(dib); + unsigned dst_pitch = FreeImage_GetPitch(dib); uint32 tileRowSize = (uint32)TIFFTileRowSize(tif); uint32 imageRowSize = (uint32)TIFFScanlineSize(tif); @@ -2088,7 +2133,7 @@ Load(FreeImageIO *io, fi_handle handle, int page, int flags, void *data) { BYTE *src_bits = tileBuffer; BYTE *dst_bits = bits + rowSize; for(int k = 0; k < nrows; k++) { - memcpy(dst_bits, src_bits, src_line); + memcpy(dst_bits, src_bits, MIN(dst_pitch, src_line)); src_bits += tileRowSize; dst_bits -= dst_pitch; }