From 00aeede6bdba3cb74943932b24accc7ba61d2cb0 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Sat, 10 Aug 2019 18:25:03 +0200 Subject: [PATCH] (CVE-2019-14973) Fix integer overflow in _TIFFCheckMalloc() and other implementation-defined behaviour (CVE-2019-14973) _TIFFCheckMalloc()/_TIFFCheckRealloc() used a unsafe way to detect overflow in the multiplication of nmemb and elem_size (which are of type tmsize_t, thus signed), which was especially easily triggered on 32-bit builds (with recent enough compilers that assume that signed multiplication cannot overflow, since this is undefined behaviour by the C standard). The original issue which lead to this fix was trigged from tif_fax3.c There were also unsafe (implementation defied), and broken in practice on 64bit builds, ways of checking that a uint64 fits of a (signed) tmsize_t by doing (uint64)(tmsize_t)uint64_var != uint64_var comparisons. Those have no known at that time exploits, but are better to fix in a more bullet-proof way. Or similarly use of (int64)uint64_var <= 0. (cherry picked from commit 1b5e3b6a23827c33acf19ad50ce5ce78f12b3773) --- libtiff/tif_aux.c | 49 +++++++++++++++++++++++++++++++++++++----- libtiff/tif_getimage.c | 6 ++---- libtiff/tif_luv.c | 8 +------ libtiff/tif_pixarlog.c | 7 +----- libtiff/tif_read.c | 38 +++++++++----------------------- libtiff/tif_strip.c | 35 ++++-------------------------- libtiff/tif_tile.c | 27 +++-------------------- libtiff/tiffiop.h | 7 +++++- 8 files changed, 71 insertions(+), 106 deletions(-) diff --git a/libtiff/tif_aux.c b/libtiff/tif_aux.c index 10b8d00c..38a98b67 100644 --- a/libtiff/tif_aux.c +++ b/libtiff/tif_aux.c @@ -59,18 +59,57 @@ _TIFFMultiply64(TIFF* tif, uint64 first, uint64 second, const char* where) return bytes; } +tmsize_t +_TIFFMultiplySSize(TIFF* tif, tmsize_t first, tmsize_t second, const char* where) +{ + if( first <= 0 || second <= 0 ) + { + if( tif != NULL && where != NULL ) + { + TIFFErrorExt(tif->tif_clientdata, where, + "Invalid argument to _TIFFMultiplySSize() in %s", where); + } + return 0; + } + + if( first > TIFF_TMSIZE_T_MAX / second ) + { + if( tif != NULL && where != NULL ) + { + TIFFErrorExt(tif->tif_clientdata, where, + "Integer overflow in %s", where); + } + return 0; + } + return first * second; +} + +tmsize_t _TIFFCastUInt64ToSSize(TIFF* tif, uint64 val, const char* module) +{ + if( val > (uint64)TIFF_TMSIZE_T_MAX ) + { + if( tif != NULL && module != NULL ) + { + TIFFErrorExt(tif->tif_clientdata,module,"Integer overflow"); + } + return 0; + } + return (tmsize_t)val; +} + 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; - + tmsize_t count = _TIFFMultiplySSize(tif, nmemb, elem_size, NULL); /* - * XXX: Check for integer overflow. + * Check for integer overflow. */ - if (nmemb && elem_size && bytes / elem_size == nmemb) - cp = _TIFFrealloc(buffer, bytes); + if (count != 0) + { + cp = _TIFFrealloc(buffer, count); + } if (cp == NULL) { TIFFErrorExt(tif->tif_clientdata, tif->tif_name, diff --git a/libtiff/tif_getimage.c b/libtiff/tif_getimage.c index fc554cca..ec09feaf 100644 --- a/libtiff/tif_getimage.c +++ b/libtiff/tif_getimage.c @@ -757,9 +757,8 @@ gtTileSeparate(TIFFRGBAImage* img, uint32* raster, uint32 w, uint32 h) uint32 leftmost_tw; tilesize = TIFFTileSize(tif); - bufsize = TIFFSafeMultiply(tmsize_t,alpha?4:3,tilesize); + bufsize = _TIFFMultiplySSize(tif, alpha?4:3,tilesize, "gtTileSeparate"); if (bufsize == 0) { - TIFFErrorExt(tif->tif_clientdata, TIFFFileName(tif), "Integer overflow in %s", "gtTileSeparate"); return (0); } @@ -1021,9 +1020,8 @@ gtStripSeparate(TIFFRGBAImage* img, uint32* raster, uint32 w, uint32 h) uint16 colorchannels; stripsize = TIFFStripSize(tif); - bufsize = TIFFSafeMultiply(tmsize_t,alpha?4:3,stripsize); + bufsize = _TIFFMultiplySSize(tif,alpha?4:3,stripsize, "gtStripSeparate"); if (bufsize == 0) { - TIFFErrorExt(tif->tif_clientdata, TIFFFileName(tif), "Integer overflow in %s", "gtStripSeparate"); return (0); } diff --git a/libtiff/tif_luv.c b/libtiff/tif_luv.c index 4b25244b..c4cb73a3 100644 --- a/libtiff/tif_luv.c +++ b/libtiff/tif_luv.c @@ -1264,16 +1264,10 @@ LogL16GuessDataFmt(TIFFDirectory *td) return (SGILOGDATAFMT_UNKNOWN); } - -#define TIFF_SIZE_T_MAX ((size_t) ~ ((size_t)0)) -#define TIFF_TMSIZE_T_MAX (tmsize_t)(TIFF_SIZE_T_MAX >> 1) - static tmsize_t multiply_ms(tmsize_t m1, tmsize_t m2) { - if( m1 == 0 || m2 > TIFF_TMSIZE_T_MAX / m1 ) - return 0; - return m1 * m2; + return _TIFFMultiplySSize(NULL, m1, m2, NULL); } static int diff --git a/libtiff/tif_pixarlog.c b/libtiff/tif_pixarlog.c index 979858da..8e9eaa1d 100644 --- a/libtiff/tif_pixarlog.c +++ b/libtiff/tif_pixarlog.c @@ -636,15 +636,10 @@ PixarLogGuessDataFmt(TIFFDirectory *td) return guess; } -#define TIFF_SIZE_T_MAX ((size_t) ~ ((size_t)0)) -#define TIFF_TMSIZE_T_MAX (tmsize_t)(TIFF_SIZE_T_MAX >> 1) - static tmsize_t multiply_ms(tmsize_t m1, tmsize_t m2) { - if( m1 == 0 || m2 > TIFF_TMSIZE_T_MAX / m1 ) - return 0; - return m1 * m2; + return _TIFFMultiplySSize(NULL, m1, m2, NULL); } static tmsize_t diff --git a/libtiff/tif_read.c b/libtiff/tif_read.c index 04100f4d..9a0e6e95 100644 --- a/libtiff/tif_read.c +++ b/libtiff/tif_read.c @@ -31,9 +31,6 @@ #include "tiffiop.h" #include -#define TIFF_SIZE_T_MAX ((size_t) ~ ((size_t)0)) -#define TIFF_TMSIZE_T_MAX (tmsize_t)(TIFF_SIZE_T_MAX >> 1) - int TIFFFillStrip(TIFF* tif, uint32 strip); int TIFFFillTile(TIFF* tif, uint32 tile); static int TIFFStartStrip(TIFF* tif, uint32 strip); @@ -51,6 +48,8 @@ TIFFReadRawTile1(TIFF* tif, uint32 tile, void* buf, tmsize_t size, const char* m #define THRESHOLD_MULTIPLIER 10 #define MAX_THRESHOLD (THRESHOLD_MULTIPLIER * THRESHOLD_MULTIPLIER * THRESHOLD_MULTIPLIER * INITIAL_THRESHOLD) +#define TIFF_INT64_MAX ((((int64)0x7FFFFFFF) << 32) | 0xFFFFFFFF) + /* Read 'size' bytes in tif_rawdata buffer starting at offset 'rawdata_offset' * Returns 1 in case of success, 0 otherwise. */ static int TIFFReadAndRealloc( TIFF* tif, tmsize_t size, @@ -735,23 +734,8 @@ TIFFReadRawStrip(TIFF* tif, uint32 strip, void* buf, tmsize_t size) return ((tmsize_t)(-1)); } bytecount = td->td_stripbytecount[strip]; - if ((int64)bytecount <= 0) { -#if defined(__WIN32__) && (defined(_MSC_VER) || defined(__MINGW32__)) - TIFFErrorExt(tif->tif_clientdata, module, - "%I64u: Invalid strip byte count, strip %lu", - (unsigned __int64) bytecount, - (unsigned long) strip); -#else - TIFFErrorExt(tif->tif_clientdata, module, - "%llu: Invalid strip byte count, strip %lu", - (unsigned long long) bytecount, - (unsigned long) strip); -#endif - return ((tmsize_t)(-1)); - } - bytecountm = (tmsize_t)bytecount; - if ((uint64)bytecountm!=bytecount) { - TIFFErrorExt(tif->tif_clientdata, module, "Integer overflow"); + bytecountm = _TIFFCastUInt64ToSSize(tif, bytecount, module); + if (bytecountm == 0) { return ((tmsize_t)(-1)); } if (size != (tmsize_t)(-1) && size < bytecountm) @@ -775,7 +759,7 @@ TIFFFillStrip(TIFF* tif, uint32 strip) if ((tif->tif_flags&TIFF_NOREADRAW)==0) { uint64 bytecount = td->td_stripbytecount[strip]; - if ((int64)bytecount <= 0) { + if( bytecount == 0 || bytecount > (uint64)TIFF_INT64_MAX ) { #if defined(__WIN32__) && (defined(_MSC_VER) || defined(__MINGW32__)) TIFFErrorExt(tif->tif_clientdata, module, "Invalid strip byte count %I64u, strip %lu", @@ -802,7 +786,7 @@ TIFFFillStrip(TIFF* tif, uint32 strip) (bytecount - 4096) / 10 > (uint64)stripsize ) { uint64 newbytecount = (uint64)stripsize * 10 + 4096; - if( (int64)newbytecount >= 0 ) + if( newbytecount == 0 || newbytecount > (uint64)TIFF_INT64_MAX ) { #if defined(__WIN32__) && (defined(_MSC_VER) || defined(__MINGW32__)) TIFFWarningExt(tif->tif_clientdata, module, @@ -1197,10 +1181,8 @@ TIFFReadRawTile(TIFF* tif, uint32 tile, void* buf, tmsize_t size) bytecount64 = td->td_stripbytecount[tile]; if (size != (tmsize_t)(-1) && (uint64)size < bytecount64) bytecount64 = (uint64)size; - bytecountm = (tmsize_t)bytecount64; - if ((uint64)bytecountm!=bytecount64) - { - TIFFErrorExt(tif->tif_clientdata,module,"Integer overflow"); + bytecountm = _TIFFCastUInt64ToSSize(tif, bytecount64, module); + if( bytecountm == 0 ) { return ((tmsize_t)(-1)); } return (TIFFReadRawTile1(tif, tile, buf, bytecountm, module)); @@ -1222,7 +1204,7 @@ TIFFFillTile(TIFF* tif, uint32 tile) if ((tif->tif_flags&TIFF_NOREADRAW)==0) { uint64 bytecount = td->td_stripbytecount[tile]; - if ((int64)bytecount <= 0) { + if( bytecount == 0 || bytecount > (uint64)TIFF_INT64_MAX ) { #if defined(__WIN32__) && (defined(_MSC_VER) || defined(__MINGW32__)) TIFFErrorExt(tif->tif_clientdata, module, "%I64u: Invalid tile byte count, tile %lu", @@ -1249,7 +1231,7 @@ TIFFFillTile(TIFF* tif, uint32 tile) (bytecount - 4096) / 10 > (uint64)stripsize ) { uint64 newbytecount = (uint64)stripsize * 10 + 4096; - if( (int64)newbytecount >= 0 ) + if( newbytecount == 0 || newbytecount > (uint64)TIFF_INT64_MAX ) { #if defined(__WIN32__) && (defined(_MSC_VER) || defined(__MINGW32__)) TIFFWarningExt(tif->tif_clientdata, module, diff --git a/libtiff/tif_strip.c b/libtiff/tif_strip.c index 6e9f2ef6..321ad6b9 100644 --- a/libtiff/tif_strip.c +++ b/libtiff/tif_strip.c @@ -131,15 +131,8 @@ TIFFVStripSize(TIFF* tif, uint32 nrows) { static const char module[] = "TIFFVStripSize"; uint64 m; - tmsize_t n; m=TIFFVStripSize64(tif,nrows); - n=(tmsize_t)m; - if ((uint64)n!=m) - { - TIFFErrorExt(tif->tif_clientdata,module,"Integer overflow"); - n=0; - } - return(n); + return _TIFFCastUInt64ToSSize(tif, m, module); } /* @@ -213,15 +206,8 @@ TIFFStripSize(TIFF* tif) { static const char module[] = "TIFFStripSize"; uint64 m; - tmsize_t n; m=TIFFStripSize64(tif); - n=(tmsize_t)m; - if ((uint64)n!=m) - { - TIFFErrorExt(tif->tif_clientdata,module,"Integer overflow"); - n=0; - } - return(n); + return _TIFFCastUInt64ToSSize(tif, m, module); } /* @@ -332,14 +318,8 @@ TIFFScanlineSize(TIFF* tif) { static const char module[] = "TIFFScanlineSize"; uint64 m; - tmsize_t n; m=TIFFScanlineSize64(tif); - n=(tmsize_t)m; - if ((uint64)n!=m) { - TIFFErrorExt(tif->tif_clientdata,module,"Integer arithmetic overflow"); - n=0; - } - return(n); + return _TIFFCastUInt64ToSSize(tif, m, module); } /* @@ -368,15 +348,8 @@ TIFFRasterScanlineSize(TIFF* tif) { static const char module[] = "TIFFRasterScanlineSize"; uint64 m; - tmsize_t n; m=TIFFRasterScanlineSize64(tif); - n=(tmsize_t)m; - if ((uint64)n!=m) - { - TIFFErrorExt(tif->tif_clientdata,module,"Integer arithmetic overflow"); - n=0; - } - return(n); + return _TIFFCastUInt64ToSSize(tif, m, module); } /* vim: set ts=8 sts=8 sw=8 noet: */ diff --git a/libtiff/tif_tile.c b/libtiff/tif_tile.c index 388e168a..7d057509 100644 --- a/libtiff/tif_tile.c +++ b/libtiff/tif_tile.c @@ -183,15 +183,8 @@ TIFFTileRowSize(TIFF* tif) { static const char module[] = "TIFFTileRowSize"; uint64 m; - tmsize_t n; m=TIFFTileRowSize64(tif); - n=(tmsize_t)m; - if ((uint64)n!=m) - { - TIFFErrorExt(tif->tif_clientdata,module,"Integer overflow"); - n=0; - } - return(n); + return _TIFFCastUInt64ToSSize(tif, m, module); } /* @@ -250,15 +243,8 @@ TIFFVTileSize(TIFF* tif, uint32 nrows) { static const char module[] = "TIFFVTileSize"; uint64 m; - tmsize_t n; m=TIFFVTileSize64(tif,nrows); - n=(tmsize_t)m; - if ((uint64)n!=m) - { - TIFFErrorExt(tif->tif_clientdata,module,"Integer overflow"); - n=0; - } - return(n); + return _TIFFCastUInt64ToSSize(tif, m, module); } /* @@ -274,15 +260,8 @@ TIFFTileSize(TIFF* tif) { static const char module[] = "TIFFTileSize"; uint64 m; - tmsize_t n; m=TIFFTileSize64(tif); - n=(tmsize_t)m; - if ((uint64)n!=m) - { - TIFFErrorExt(tif->tif_clientdata,module,"Integer overflow"); - n=0; - } - return(n); + return _TIFFCastUInt64ToSSize(tif, m, module); } /* diff --git a/libtiff/tiffiop.h b/libtiff/tiffiop.h index 08e5dc44..d4b86314 100644 --- a/libtiff/tiffiop.h +++ b/libtiff/tiffiop.h @@ -79,6 +79,9 @@ extern int snprintf(char* str, size_t size, const char* format, ...); #define FALSE 0 #endif +#define TIFF_SIZE_T_MAX ((size_t) ~ ((size_t)0)) +#define TIFF_TMSIZE_T_MAX (tmsize_t)(TIFF_SIZE_T_MAX >> 1) + typedef struct client_info { struct client_info *next; void *data; @@ -260,7 +263,7 @@ struct tiff { #define TIFFhowmany8_64(x) (((x)&0x07)?((uint64)(x)>>3)+1:(uint64)(x)>>3) #define TIFFroundup_64(x, y) (TIFFhowmany_64(x,y)*(y)) -/* Safe multiply which returns zero if there is an integer overflow */ +/* Safe multiply which returns zero if there is an *unsigned* integer overflow. This macro is not safe for *signed* integer types */ #define TIFFSafeMultiply(t,v,m) ((((t)(m) != (t)0) && (((t)(((v)*(m))/(m))) == (t)(v))) ? (t)((v)*(m)) : (t)0) #define TIFFmax(A,B) ((A)>(B)?(A):(B)) @@ -366,6 +369,8 @@ extern TIFFErrorHandlerExt _TIFFerrorHandlerExt; extern uint32 _TIFFMultiply32(TIFF*, uint32, uint32, const char*); extern uint64 _TIFFMultiply64(TIFF*, uint64, uint64, const char*); +extern tmsize_t _TIFFMultiplySSize(TIFF*, tmsize_t, tmsize_t, const char*); +extern tmsize_t _TIFFCastUInt64ToSSize(TIFF*, uint64, const char*); extern void* _TIFFCheckMalloc(TIFF*, tmsize_t, tmsize_t, const char*); extern void* _TIFFCheckRealloc(TIFF*, void*, tmsize_t, tmsize_t, const char*);