From 1b5e3b6a23827c33acf19ad50ce5ce78f12b3773 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Sat, 10 Aug 2019 18:25:03 +0200 Subject: 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. --- libtiff/tif_aux.c | 49 ++++++++++++++++++++++++++++++++++++++++++++----- libtiff/tif_getimage.c | 6 ++---- libtiff/tif_luv.c | 8 +------- libtiff/tif_pixarlog.c | 8 +------- libtiff/tif_read.c | 38 ++++++++++---------------------------- libtiff/tif_strip.c | 35 ++++------------------------------- libtiff/tif_tile.c | 27 +++------------------------ libtiff/tiffiop.h | 7 ++++++- 8 files changed, 71 insertions(+), 107 deletions(-) diff --git a/libtiff/tif_aux.c b/libtiff/tif_aux.c index 90d30214..3e9bda43 100644 --- a/libtiff/tif_aux.c +++ b/libtiff/tif_aux.c @@ -58,18 +58,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 599bf127..c88b5fa6 100644 --- a/libtiff/tif_getimage.c +++ b/libtiff/tif_getimage.c @@ -755,9 +755,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); } @@ -1019,9 +1018,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 6a63eadc..6fe48588 100644 --- a/libtiff/tif_luv.c +++ b/libtiff/tif_luv.c @@ -1269,16 +1269,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 b1e48d99..a2149c85 100644 --- a/libtiff/tif_pixarlog.c +++ b/libtiff/tif_pixarlog.c @@ -634,16 +634,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) { - assert(m1 >= 0 && m2 >= 0); - 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 1f5362a7..c436b0d5 100644 --- a/libtiff/tif_read.c +++ b/libtiff/tif_read.c @@ -29,9 +29,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); @@ -49,6 +46,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, @@ -727,23 +726,8 @@ TIFFReadRawStrip(TIFF* tif, uint32 strip, void* buf, tmsize_t size) return ((tmsize_t)(-1)); } bytecount = TIFFGetStrileByteCount(tif, 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) @@ -764,7 +748,7 @@ TIFFFillStrip(TIFF* tif, uint32 strip) if ((tif->tif_flags&TIFF_NOREADRAW)==0) { uint64 bytecount = TIFFGetStrileByteCount(tif, 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", @@ -791,7 +775,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, @@ -1181,10 +1165,8 @@ TIFFReadRawTile(TIFF* tif, uint32 tile, void* buf, tmsize_t size) bytecount64 = TIFFGetStrileByteCount(tif, 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)); @@ -1203,7 +1185,7 @@ TIFFFillTile(TIFF* tif, uint32 tile) if ((tif->tif_flags&TIFF_NOREADRAW)==0) { uint64 bytecount = TIFFGetStrileByteCount(tif, 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", @@ -1230,7 +1212,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 2f3cd883..c08c60a7 100644 --- a/libtiff/tif_strip.c +++ b/libtiff/tif_strip.c @@ -129,15 +129,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); } /* @@ -210,15 +203,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); } /* @@ -329,14 +315,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); } /* @@ -365,15 +345,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 58fe9354..661cc771 100644 --- a/libtiff/tif_tile.c +++ b/libtiff/tif_tile.c @@ -181,15 +181,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); } /* @@ -248,15 +241,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); } /* @@ -272,15 +258,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 dd6fb095..60ccc87b 100644 --- a/libtiff/tiffiop.h +++ b/libtiff/tiffiop.h @@ -77,6 +77,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; @@ -261,7 +264,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)) @@ -371,6 +374,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*); -- cgit v1.2.1