CVE-2018-7456: NULL Pointer Dereference (CWE-476)
value: A NULL Pointer Dereference occurs in the function TIFFPrintDirectory in tif_print.c in LibTIFF 3.9.3, 3.9.4, 3.9.5, 3.9.6, 3.9.7, 4.0.0alpha4, 4.0.0alpha5, 4.0.0alpha6, 4.0.0beta7, 4.0.0, 4.0.1, 4.0.2, 4.0.3, 4.0.4, 4.0.4beta, 4.0.5, 4.0.6, 4.0.7, 4.0.8 and 4.0.9 when using the tiffinfo tool to print crafted TIFF information, a different vulnerability than CVE-2017-18013. (This affects an earlier part of the TIFFPrintDirectory function that was not addressed by the CVE-2017-18013 patch.)
CWE description: The product dereferences a pointer that it expects to be valid but is NULL.
CVSS v3 base score: 6.5
Published: 2018-02-24T06:29Z
Repository: https://gitlab.com/libtiff/libtiff
Language: C
Files changed: libtiff/tif_dirread.c, libtiff/tif_print.c
Commit date: 2018-04-11 23:09:59 -0400
Commit message: 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.

Diff: diff --git a/libtiff/tif_dirread.c b/libtiff/tif_dirread.c
index 6baa7b31..af5b84a7 100644
--- a/libtiff/tif_dirread.c
+++ b/libtiff/tif_dirread.c
@@ -163,10 +163,11 @@ static uint16 TIFFFetchDirectory(TIFF* tif, uint64 diroff, TIFFDirEntry** pdir,
 static int TIFFFetchNormalTag(TIFF*, TIFFDirEntry*, int recover);
 static int TIFFFetchStripThing(TIFF* tif, TIFFDirEntry* dir, uint32 nstrips, uint64** lpp);
 static int TIFFFetchSubjectDistance(TIFF*, TIFFDirEntry*);
 static void ChopUpSingleUncompressedStrip(TIFF*);
 static uint64 TIFFReadUInt64(const uint8 *value);
+static int _TIFFGetMaxColorChannels(uint16 photometric);
 
 static int _TIFFFillStrilesInternal( TIFF *tif, int loadStripByteCount );
 
 typedef union _UInt64Aligned_t
 {
@@ -3502,10 +3503,39 @@ static void TIFFReadDirEntryOutputErr(TIFF* tif, enum TIFFReadDirEntryErr err, c
 				break;
 		}
 	}
 }
 
+/*
+ * 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_CFA:
+	case PHOTOMETRIC_ITULAB:
+	case PHOTOMETRIC_ICCLAB:
+	default:
+            return 0;
+    }
+}
+
 /*
  * Read the next TIFF directory from a file and convert it to the internal
  * format. We read directories sequentially.
  */
 int
@@ -3518,10 +3548,11 @@ TIFFReadDirectory(TIFF* tif)
 	uint16 di;
 	const TIFFField* fip;
 	uint32 fii=FAILED_FII;
         toff_t nextdiroff;
     int bitspersample_read = FALSE;
+        int color_channels;
 
 	tif->tif_diroff=tif->tif_nextdiroff;
 	if (!TIFFCheckDirOffset(tif,tif->tif_nextdiroff))
 		return 0;           /* last offset or bad offset (IFD looping) */
 	(*tif->tif_cleanup)(tif);   /* cleanup any previous compression state */
@@ -4022,10 +4053,41 @@ TIFFReadDirectory(TIFF* tif)
 				if (!TIFFSetField(tif,TIFFTAG_SAMPLESPERPIXEL,1))
 					goto bad;
 			}
 		}
 	}
+
+	/*
+	 * 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.
 	 */
 	if (tif->tif_dir.td_photometric == PHOTOMETRIC_PALETTE &&
 	    !TIFFFieldSet(tif, FIELD_COLORMAP)) {
diff --git a/libtiff/tif_print.c b/libtiff/tif_print.c
index 8deceb2b..1d86adbf 100644
--- a/libtiff/tif_print.c
+++ b/libtiff/tif_print.c
@@ -542,11 +542,11 @@ TIFFPrintDirectory(TIFF* tif, FILE* fd, long flags)
 			n = 1L<<td->td_bitspersample;
 			for (l = 0; l < n; l++) {
 				uint16 i;
 				fprintf(fd, "    %2ld: %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);
 			}
 		} else

Vulnerable code: 				for (i = 1; i < td->td_samplesperpixel; i++)
Fixed code: 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_CFA:
	case PHOTOMETRIC_ITULAB:
	case PHOTOMETRIC_ICCLAB:
	default:
            return 0;
    }
}

        int color_channels;

	/*
	 * 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);
        }

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