From f1c2759436a0ee5181cae7176114f57e45f0eb16 Mon Sep 17 00:00:00 2001

From: Hugo Lefeuvre <hle@debian.org>

Date: Sun, 8 Apr 2018 14:07:08 -0400

Subject: [PATCH] Fix NULL pointer dereference in TIFFPrintDirectory

The TIFFPrintDirectory function relies on the following assumptions,

supposed to be guaranteed by the specification:

(a) A Transfer Function field is only present if the TIFF file has

    photometric type < 3.

(b) If SamplesPerPixel > Color Channels, then the ExtraSamples field

    has count SamplesPerPixel - (Color Channels) and contains

    information about supplementary channels.

While respect of (a) and (b) are essential for the well functioning of

TIFFPrintDirectory, no checks are realized neither by the callee nor

by TIFFPrintDirectory itself. Hence, following scenarios might happen

and trigger the NULL pointer dereference:

(1) TIFF File of photometric type 4 or more has illegal Transfer

    Function field.

(2) TIFF File has photometric type 3 or less and defines a

    SamplesPerPixel field such that SamplesPerPixel > Color Channels

    without defining all extra samples in the ExtraSamples fields.

In this patch, we address both issues with respect of the following

principles:

(A) In the case of (1), the defined transfer table should be printed

    safely even if it isn't 'legal'. This allows us to avoid expensive

    checks in TIFFPrintDirectory. Also, it is quite possible that

    an alternative photometric type would be developed (not part of the

    standard) and would allow definition of Transfer Table. We want

    libtiff to be able to handle this scenario out of the box.

(B) In the case of (2), the transfer table should be printed at its

    right size, that is if TIFF file has photometric type Palette

    then the transfer table should have one row and not three, even

    if two extra samples are declared.

In order to fulfill (A) we simply add a new 'i < 3' end condition to

the broken TIFFPrintDirectory loop. This makes sure that in any case

where (b) would be respected but not (a), everything stays fine.

(B) is fulfilled by the loop condition

'i < td->td_samplesperpixel - td->td_extrasamples'. This is enough as

long as (b) is respected.

Naturally, we also make sure (b) is respected. This is done in the

TIFFReadDirectory function by making sure any non-color channel is

counted in ExtraSamples.

This commit addresses CVE-2018-7456.

---

 libtiff/tif_dirread.c | 61 +++++++++++++++++++++++++++++++++++++++++++

 libtiff/tif_print.c   |  2 +-

 libtiff/tif_unix.c    |  9 +++++++

 libtiff/tiffio.h      |  1 +

 4 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/libtiff/tif_dirread.c b/libtiff/tif_dirread.c

index 0a9484d..8dc40b5 100644

--- a/libtiff/tif_dirread.c

+++ b/libtiff/tif_dirread.c

@@ -65,6 +65,35 @@ static	int TIFFFetchDoubleArray(TIFF*, TIFFDirEntry*, double*);

 static	int TIFFFetchAnyArray(TIFF*, TIFFDirEntry*, double*);

 static	int TIFFFetchShortPair(TIFF*, TIFFDirEntry*);

 static	void ChopUpSingleUncompressedStrip(TIFF*);

+static	int _TIFFGetMaxColorChannels(uint16 photometric);

+

+/*

+ * Return the maximum number of color channels specified for a given photometric

+ * type. 0 is returned if photometric type isn't supported or no default value

+ * is defined by the specification.

+ */

+static int _TIFFGetMaxColorChannels( uint16 photometric )

+{

+    switch (photometric) {

+	case PHOTOMETRIC_PALETTE:

+	case PHOTOMETRIC_MINISWHITE:

+	case PHOTOMETRIC_MINISBLACK:

+            return 1;

+	case PHOTOMETRIC_YCBCR:

+	case PHOTOMETRIC_RGB:

+	case PHOTOMETRIC_CIELAB:

+            return 3;

+	case PHOTOMETRIC_SEPARATED:

+	case PHOTOMETRIC_MASK:

+            return 4;

+	case PHOTOMETRIC_LOGL:

+	case PHOTOMETRIC_LOGLUV:

+	case PHOTOMETRIC_ITULAB:

+	case PHOTOMETRIC_ICCLAB:

+	default:

+            return 0;

+    }

+}

 /*

  * Read the next TIFF directory from a file and convert it to the internal

@@ -86,6 +115,7 @@ TIFFReadDirectory(TIFF* tif)

 	uint16 previous_tag = 0;

 	int diroutoforderwarning = 0, compressionknown = 0;

 	int haveunknowntags = 0;

+	int color_channels;

 	tif->tif_diroff = tif->tif_nextdiroff;

 	/*

@@ -617,6 +647,37 @@ TIFFReadDirectory(TIFF* tif)

 			}

 		}

 	}

+

+	/*

+	 * Make sure all non-color channels are extrasamples.

+	 * If it's not the case, define them as such.

+	 */

+        color_channels = _TIFFGetMaxColorChannels(tif->tif_dir.td_photometric);

+        if (color_channels && tif->tif_dir.td_samplesperpixel - tif->tif_dir.td_extrasamples > color_channels) {

+                uint16 old_extrasamples;

+                uint16 *new_sampleinfo;

+

+                TIFFWarningExt(tif->tif_clientdata,module, "Sum of Photometric type-related "

+                    "color channels and ExtraSamples doesn't match SamplesPerPixel. "

+                    "Defining non-color channels as ExtraSamples.");

+

+                old_extrasamples = tif->tif_dir.td_extrasamples;

+                tif->tif_dir.td_extrasamples = (tif->tif_dir.td_samplesperpixel - color_channels);

+

+                // sampleinfo should contain information relative to these new extra samples

+                new_sampleinfo = (uint16*) _TIFFcalloc(tif->tif_dir.td_extrasamples, sizeof(uint16));

+                if (!new_sampleinfo) {

+                    TIFFErrorExt(tif->tif_clientdata, module, "Failed to allocate memory for "

+                                "temporary new sampleinfo array (%d 16 bit elements)",

+                                tif->tif_dir.td_extrasamples);

+                    goto bad;

+                }

+

+                memcpy(new_sampleinfo, tif->tif_dir.td_sampleinfo, old_extrasamples * sizeof(uint16));

+                _TIFFsetShortArray(&tif->tif_dir.td_sampleinfo, new_sampleinfo, tif->tif_dir.td_extrasamples);

+                _TIFFfree(new_sampleinfo);

+        }

+

 	/*

 	 * Verify Palette image has a Colormap.

 	 */

diff --git a/libtiff/tif_print.c b/libtiff/tif_print.c

index 0f6ea01..1ec4f26 100644

--- a/libtiff/tif_print.c

+++ b/libtiff/tif_print.c

@@ -503,7 +503,7 @@ TIFFPrintDirectory(TIFF* tif, FILE* fd, long flags)

 			for (l = 0; l < n; l++) {

 				fprintf(fd, "    %2lu: %5u",

 				    l, td->td_transferfunction[0][l]);

-				for (i = 1; i < td->td_samplesperpixel; i++)

+				for (i = 1; i < td->td_samplesperpixel - td->td_extrasamples && i < 3; i++)

 					fprintf(fd, " %5u",

 					    td->td_transferfunction[i][l]);

 				fputc('\n', fd);

diff --git a/libtiff/tif_unix.c b/libtiff/tif_unix.c

index b73e80d..5d29040 100644

--- a/libtiff/tif_unix.c

+++ b/libtiff/tif_unix.c

@@ -241,6 +241,15 @@ _TIFFmalloc(tsize_t s)

 	return (malloc((size_t) s));

 }

+void*

+_TIFFcalloc(tsize_t nmemb, tsize_t siz)

+{

+    if( nmemb == 0 || siz == 0 )

+        return ((void *) NULL);

+

+    return calloc((size_t) nmemb, (size_t)siz);

+}

+

 void

 _TIFFfree(tdata_t p)

 {

diff --git a/libtiff/tiffio.h b/libtiff/tiffio.h

index 06ec25c..3cf8e75 100644

--- a/libtiff/tiffio.h

+++ b/libtiff/tiffio.h

@@ -281,6 +281,7 @@ extern	TIFFCodec* TIFFGetConfiguredCODECs(void);

  */

 extern	tdata_t _TIFFmalloc(tsize_t);

+extern	tdata_t _TIFFcalloc(tsize_t, tsize_t);

 extern	tdata_t _TIFFrealloc(tdata_t, tsize_t);

 extern	void _TIFFmemset(tdata_t, int, tsize_t);

 extern	void _TIFFmemcpy(tdata_t, const tdata_t, tsize_t);

-- 

2.17.0