197 lines
7.1 KiB
Diff
197 lines
7.1 KiB
Diff
|
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
|
||
|
|