From 0f797bf3863011d0fdc1e3fb7cfe802b64d9b19c Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Fri, 24 May 2019 11:20:27 +0200 Subject: [PATCH] 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. --- libtiff/tif_dirread.c | 8 +++++--- libtiff/tiffiop.h | 1 + 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/libtiff/tif_dirread.c b/libtiff/tif_dirread.c index f5432e84..77e1d9a7 100644 --- a/libtiff/tif_dirread.c +++ b/libtiff/tif_dirread.c @@ -3625,6 +3625,8 @@ TIFFReadDirectory(TIFF* tif) tif->tif_flags &= ~TIFF_BEENWRITING; /* reset before new dir */ tif->tif_flags &= ~TIFF_BUF4WRITE; /* reset before new dir */ + tif->tif_flags &= ~TIFF_CHOPPEDUPARRAYS; + /* free any old stuff and reinit */ TIFFFreeDirectory(tif); TIFFDefaultDirectory(tif); @@ -5778,7 +5780,7 @@ static void allocChoppedUpStripArrays(TIFF* tif, uint32 nstrips, #ifdef STRIPBYTECOUNTSORTED_UNUSED td->td_stripbytecountsorted = 1; #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; if( pbErr ) *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 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; /* 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; if( tif->tif_flags&TIFF_LAZYSTRILELOAD ) diff --git a/libtiff/tiffiop.h b/libtiff/tiffiop.h index ef63de53..dd6fb095 100644 --- a/libtiff/tiffiop.h +++ b/libtiff/tiffiop.h @@ -129,6 +129,7 @@ struct tiff { #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_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_nextdiroff; /* file offset of following directory */ uint64* tif_dirlist; /* list of offsets to already seen directories to prevent IFD looping */