Fix vulnerability introduced by defer strile loading (master only)

Found on GDAL with https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=14894
Disabling the TIFF_DEFERSTRILELOAD bit in ChopupStripArray() was a
bad idea since when using TIFFReadDirectory() to reload the directory again
would lead to a different value of td_rowsperstrip, which could confuse
readers if they relied on the value found initially.
This commit is contained in:
Even Rouault 2019-05-24 11:20:27 +02:00
parent fdec42f217
commit 0f797bf386
No known key found for this signature in database
GPG Key ID: 33EBBFC47B3DD87D
2 changed files with 6 additions and 3 deletions

View File

@ -3625,6 +3625,8 @@ TIFFReadDirectory(TIFF* tif)
tif->tif_flags &= ~TIFF_BEENWRITING; /* reset before new dir */ tif->tif_flags &= ~TIFF_BEENWRITING; /* reset before new dir */
tif->tif_flags &= ~TIFF_BUF4WRITE; /* reset before new dir */ tif->tif_flags &= ~TIFF_BUF4WRITE; /* reset before new dir */
tif->tif_flags &= ~TIFF_CHOPPEDUPARRAYS;
/* free any old stuff and reinit */ /* free any old stuff and reinit */
TIFFFreeDirectory(tif); TIFFFreeDirectory(tif);
TIFFDefaultDirectory(tif); TIFFDefaultDirectory(tif);
@ -5778,7 +5780,7 @@ static void allocChoppedUpStripArrays(TIFF* tif, uint32 nstrips,
#ifdef STRIPBYTECOUNTSORTED_UNUSED #ifdef STRIPBYTECOUNTSORTED_UNUSED
td->td_stripbytecountsorted = 1; td->td_stripbytecountsorted = 1;
#endif #endif
tif->tif_flags &= ~TIFF_DEFERSTRILELOAD; tif->tif_flags |= TIFF_CHOPPEDUPARRAYS;
} }
@ -6196,7 +6198,7 @@ static uint64 _TIFFGetStrileOffsetOrByteCountValue(TIFF *tif, uint32 strile,
TIFFDirectory *td = &tif->tif_dir; TIFFDirectory *td = &tif->tif_dir;
if( pbErr ) if( pbErr )
*pbErr = 0; *pbErr = 0;
if( tif->tif_flags&TIFF_DEFERSTRILELOAD ) if( (tif->tif_flags&TIFF_DEFERSTRILELOAD) && !(tif->tif_flags&TIFF_CHOPPEDUPARRAYS) )
{ {
if( !(tif->tif_flags&TIFF_LAZYSTRILELOAD) || if( !(tif->tif_flags&TIFF_LAZYSTRILELOAD) ||
/* If the values may fit in the toff_long/toff_long8 member */ /* If the values may fit in the toff_long/toff_long8 member */
@ -6271,7 +6273,7 @@ static int _TIFFFillStrilesInternal( TIFF *tif, int loadStripByteCount )
int return_value = 1; int return_value = 1;
/* Do not do anything if TIFF_DEFERSTRILELOAD is not set */ /* Do not do anything if TIFF_DEFERSTRILELOAD is not set */
if( !(tif->tif_flags&TIFF_DEFERSTRILELOAD) ) if( !(tif->tif_flags&TIFF_DEFERSTRILELOAD) || (tif->tif_flags&TIFF_CHOPPEDUPARRAYS) != 0 )
return 1; return 1;
if( tif->tif_flags&TIFF_LAZYSTRILELOAD ) if( tif->tif_flags&TIFF_LAZYSTRILELOAD )

View File

@ -129,6 +129,7 @@ struct tiff {
#define TIFF_BUFFERMMAP 0x800000U /* read buffer (tif_rawdata) points into mmap() memory */ #define TIFF_BUFFERMMAP 0x800000U /* read buffer (tif_rawdata) points into mmap() memory */
#define TIFF_DEFERSTRILELOAD 0x1000000U /* defer strip/tile offset/bytecount array loading. */ #define TIFF_DEFERSTRILELOAD 0x1000000U /* defer strip/tile offset/bytecount array loading. */
#define TIFF_LAZYSTRILELOAD 0x2000000U /* lazy/ondemand loading of strip/tile offset/bytecount values. Only used if TIFF_DEFERSTRILELOAD is set and in read-only mode */ #define TIFF_LAZYSTRILELOAD 0x2000000U /* lazy/ondemand loading of strip/tile offset/bytecount values. Only used if TIFF_DEFERSTRILELOAD is set and in read-only mode */
#define TIFF_CHOPPEDUPARRAYS 0x4000000U /* set when allocChoppedUpStripArrays() has modified strip array */
uint64 tif_diroff; /* file offset of current directory */ uint64 tif_diroff; /* file offset of current directory */
uint64 tif_nextdiroff; /* file offset of following directory */ uint64 tif_nextdiroff; /* file offset of following directory */
uint64* tif_dirlist; /* list of offsets to already seen directories to prevent IFD looping */ uint64* tif_dirlist; /* list of offsets to already seen directories to prevent IFD looping */