From 218c3753fba788c78a9b5e515e884043f6e2ba28 Mon Sep 17 00:00:00 2001

From: Even Rouault <even.rouault@spatialys.com>

Date: Sat, 10 Aug 2019 18:25:03 +0200

Subject: [PATCH] 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 |  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 10b8d00..38a98b6 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 fc554cc..ec09fea 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 4b25244..c4cb73a 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 979858d..8e9eaa1 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 04100f4..9a0e6e9 100644

--- a/libtiff/tif_read.c

+++ b/libtiff/tif_read.c

@@ -31,9 +31,6 @@

 #include "tiffiop.h"

 #include <stdio.h>

-#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 6e9f2ef..321ad6b 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 388e168..7d05750 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 08e5dc4..d4b8631 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*);

-- 

2.21.0