|
|
460672 |
From 52dcdc1d89925fe7e362b2cc9cae699773f9618e Mon Sep 17 00:00:00 2001
|
|
|
460672 |
From: Hugo Lefeuvre <hle@debian.org>
|
|
|
460672 |
Date: Sun, 8 Apr 2018 14:07:08 -0400
|
|
|
460672 |
Subject: [PATCH] Fix NULL pointer dereference in TIFFPrintDirectory
|
|
|
460672 |
|
|
|
460672 |
The TIFFPrintDirectory function relies on the following assumptions,
|
|
|
460672 |
supposed to be guaranteed by the specification:
|
|
|
460672 |
|
|
|
460672 |
(a) A Transfer Function field is only present if the TIFF file has
|
|
|
460672 |
photometric type < 3.
|
|
|
460672 |
|
|
|
460672 |
(b) If SamplesPerPixel > Color Channels, then the ExtraSamples field
|
|
|
460672 |
has count SamplesPerPixel - (Color Channels) and contains
|
|
|
460672 |
information about supplementary channels.
|
|
|
460672 |
|
|
|
460672 |
While respect of (a) and (b) are essential for the well functioning of
|
|
|
460672 |
TIFFPrintDirectory, no checks are realized neither by the callee nor
|
|
|
460672 |
by TIFFPrintDirectory itself. Hence, following scenarios might happen
|
|
|
460672 |
and trigger the NULL pointer dereference:
|
|
|
460672 |
|
|
|
460672 |
(1) TIFF File of photometric type 4 or more has illegal Transfer
|
|
|
460672 |
Function field.
|
|
|
460672 |
|
|
|
460672 |
(2) TIFF File has photometric type 3 or less and defines a
|
|
|
460672 |
SamplesPerPixel field such that SamplesPerPixel > Color Channels
|
|
|
460672 |
without defining all extra samples in the ExtraSamples fields.
|
|
|
460672 |
|
|
|
460672 |
In this patch, we address both issues with respect of the following
|
|
|
460672 |
principles:
|
|
|
460672 |
|
|
|
460672 |
(A) In the case of (1), the defined transfer table should be printed
|
|
|
460672 |
safely even if it isn't 'legal'. This allows us to avoid expensive
|
|
|
460672 |
checks in TIFFPrintDirectory. Also, it is quite possible that
|
|
|
460672 |
an alternative photometric type would be developed (not part of the
|
|
|
460672 |
standard) and would allow definition of Transfer Table. We want
|
|
|
460672 |
libtiff to be able to handle this scenario out of the box.
|
|
|
460672 |
|
|
|
460672 |
(B) In the case of (2), the transfer table should be printed at its
|
|
|
460672 |
right size, that is if TIFF file has photometric type Palette
|
|
|
460672 |
then the transfer table should have one row and not three, even
|
|
|
460672 |
if two extra samples are declared.
|
|
|
460672 |
|
|
|
460672 |
In order to fulfill (A) we simply add a new 'i < 3' end condition to
|
|
|
460672 |
the broken TIFFPrintDirectory loop. This makes sure that in any case
|
|
|
460672 |
where (b) would be respected but not (a), everything stays fine.
|
|
|
460672 |
|
|
|
460672 |
(B) is fulfilled by the loop condition
|
|
|
460672 |
'i < td->td_samplesperpixel - td->td_extrasamples'. This is enough as
|
|
|
460672 |
long as (b) is respected.
|
|
|
460672 |
|
|
|
460672 |
Naturally, we also make sure (b) is respected. This is done in the
|
|
|
460672 |
TIFFReadDirectory function by making sure any non-color channel is
|
|
|
460672 |
counted in ExtraSamples.
|
|
|
460672 |
|
|
|
460672 |
This commit addresses CVE-2018-7456.
|
|
|
460672 |
---
|
|
|
460672 |
libtiff/tif_dirread.c | 61 +++++++++++++++++++++++++++++++++++++++++++
|
|
|
460672 |
libtiff/tif_print.c | 2 +-
|
|
|
460672 |
libtiff/tif_unix.c | 8 ++++++
|
|
|
460672 |
libtiff/tif_win32.c | 8 ++++++
|
|
|
460672 |
libtiff/tiffio.h | 1 +
|
|
|
460672 |
5 files changed, 79 insertions(+), 1 deletion(-)
|
|
|
460672 |
|
|
|
460672 |
diff --git a/libtiff/tif_dirread.c b/libtiff/tif_dirread.c
|
|
|
460672 |
index 6667228..4f07bc8 100644
|
|
|
460672 |
--- a/libtiff/tif_dirread.c
|
|
|
460672 |
+++ b/libtiff/tif_dirread.c
|
|
|
460672 |
@@ -165,6 +165,7 @@ static int TIFFFetchStripThing(TIFF* tif, TIFFDirEntry* dir, uint32 nstrips, uin
|
|
|
460672 |
static int TIFFFetchSubjectDistance(TIFF*, TIFFDirEntry*);
|
|
|
460672 |
static void ChopUpSingleUncompressedStrip(TIFF*);
|
|
|
460672 |
static uint64 TIFFReadUInt64(const uint8 *value);
|
|
|
460672 |
+static int _TIFFGetMaxColorChannels(uint16 photometric);
|
|
|
460672 |
|
|
|
460672 |
typedef union _UInt64Aligned_t
|
|
|
460672 |
{
|
|
|
460672 |
@@ -3415,6 +3416,34 @@ static void TIFFReadDirEntryOutputErr(TIFF* tif, enum TIFFReadDirEntryErr err, c
|
|
|
460672 |
}
|
|
|
460672 |
}
|
|
|
460672 |
|
|
|
460672 |
+/*
|
|
|
460672 |
+ * Return the maximum number of color channels specified for a given photometric
|
|
|
460672 |
+ * type. 0 is returned if photometric type isn't supported or no default value
|
|
|
460672 |
+ * is defined by the specification.
|
|
|
460672 |
+ */
|
|
|
460672 |
+static int _TIFFGetMaxColorChannels( uint16 photometric )
|
|
|
460672 |
+{
|
|
|
460672 |
+ switch (photometric) {
|
|
|
460672 |
+ case PHOTOMETRIC_PALETTE:
|
|
|
460672 |
+ case PHOTOMETRIC_MINISWHITE:
|
|
|
460672 |
+ case PHOTOMETRIC_MINISBLACK:
|
|
|
460672 |
+ return 1;
|
|
|
460672 |
+ case PHOTOMETRIC_YCBCR:
|
|
|
460672 |
+ case PHOTOMETRIC_RGB:
|
|
|
460672 |
+ case PHOTOMETRIC_CIELAB:
|
|
|
460672 |
+ return 3;
|
|
|
460672 |
+ case PHOTOMETRIC_SEPARATED:
|
|
|
460672 |
+ case PHOTOMETRIC_MASK:
|
|
|
460672 |
+ return 4;
|
|
|
460672 |
+ case PHOTOMETRIC_LOGL:
|
|
|
460672 |
+ case PHOTOMETRIC_LOGLUV:
|
|
|
460672 |
+ case PHOTOMETRIC_ITULAB:
|
|
|
460672 |
+ case PHOTOMETRIC_ICCLAB:
|
|
|
460672 |
+ default:
|
|
|
460672 |
+ return 0;
|
|
|
460672 |
+ }
|
|
|
460672 |
+}
|
|
|
460672 |
+
|
|
|
460672 |
/*
|
|
|
460672 |
* Read the next TIFF directory from a file and convert it to the internal
|
|
|
460672 |
* format. We read directories sequentially.
|
|
|
460672 |
@@ -3430,6 +3459,7 @@ TIFFReadDirectory(TIFF* tif)
|
|
|
460672 |
const TIFFField* fip;
|
|
|
460672 |
uint32 fii=FAILED_FII;
|
|
|
460672 |
toff_t nextdiroff;
|
|
|
460672 |
+ int color_channels;
|
|
|
460672 |
tif->tif_diroff=tif->tif_nextdiroff;
|
|
|
460672 |
if (!TIFFCheckDirOffset(tif,tif->tif_nextdiroff))
|
|
|
460672 |
return 0; /* last offset or bad offset (IFD looping) */
|
|
|
460672 |
@@ -3909,6 +3939,37 @@ TIFFReadDirectory(TIFF* tif)
|
|
|
460672 |
}
|
|
|
460672 |
}
|
|
|
460672 |
}
|
|
|
460672 |
+
|
|
|
460672 |
+ /*
|
|
|
460672 |
+ * Make sure all non-color channels are extrasamples.
|
|
|
460672 |
+ * If it's not the case, define them as such.
|
|
|
460672 |
+ */
|
|
|
460672 |
+ color_channels = _TIFFGetMaxColorChannels(tif->tif_dir.td_photometric);
|
|
|
460672 |
+ if (color_channels && tif->tif_dir.td_samplesperpixel - tif->tif_dir.td_extrasamples > color_channels) {
|
|
|
460672 |
+ uint16 old_extrasamples;
|
|
|
460672 |
+ uint16 *new_sampleinfo;
|
|
|
460672 |
+
|
|
|
460672 |
+ TIFFWarningExt(tif->tif_clientdata,module, "Sum of Photometric type-related "
|
|
|
460672 |
+ "color channels and ExtraSamples doesn't match SamplesPerPixel. "
|
|
|
460672 |
+ "Defining non-color channels as ExtraSamples.");
|
|
|
460672 |
+
|
|
|
460672 |
+ old_extrasamples = tif->tif_dir.td_extrasamples;
|
|
|
460672 |
+ tif->tif_dir.td_extrasamples = (tif->tif_dir.td_samplesperpixel - color_channels);
|
|
|
460672 |
+
|
|
|
460672 |
+ // sampleinfo should contain information relative to these new extra samples
|
|
|
460672 |
+ new_sampleinfo = (uint16*) _TIFFcalloc(tif->tif_dir.td_extrasamples, sizeof(uint16));
|
|
|
460672 |
+ if (!new_sampleinfo) {
|
|
|
460672 |
+ TIFFErrorExt(tif->tif_clientdata, module, "Failed to allocate memory for "
|
|
|
460672 |
+ "temporary new sampleinfo array (%d 16 bit elements)",
|
|
|
460672 |
+ tif->tif_dir.td_extrasamples);
|
|
|
460672 |
+ goto bad;
|
|
|
460672 |
+ }
|
|
|
460672 |
+
|
|
|
460672 |
+ memcpy(new_sampleinfo, tif->tif_dir.td_sampleinfo, old_extrasamples * sizeof(uint16));
|
|
|
460672 |
+ _TIFFsetShortArray(&tif->tif_dir.td_sampleinfo, new_sampleinfo, tif->tif_dir.td_extrasamples);
|
|
|
460672 |
+ _TIFFfree(new_sampleinfo);
|
|
|
460672 |
+ }
|
|
|
460672 |
+
|
|
|
460672 |
/*
|
|
|
460672 |
* Verify Palette image has a Colormap.
|
|
|
460672 |
*/
|
|
|
460672 |
diff --git a/libtiff/tif_print.c b/libtiff/tif_print.c
|
|
|
460672 |
index b323ef1..8c075e5 100644
|
|
|
460672 |
--- a/libtiff/tif_print.c
|
|
|
460672 |
+++ b/libtiff/tif_print.c
|
|
|
460672 |
@@ -541,7 +541,7 @@ TIFFPrintDirectory(TIFF* tif, FILE* fd, long flags)
|
|
|
460672 |
for (l = 0; l < n; l++) {
|
|
|
460672 |
fprintf(fd, " %2lu: %5u",
|
|
|
460672 |
l, td->td_transferfunction[0][l]);
|
|
|
460672 |
- for (i = 1; i < td->td_samplesperpixel; i++)
|
|
|
460672 |
+ for (i = 1; i < td->td_samplesperpixel - td->td_extrasamples && i < 3; i++)
|
|
|
460672 |
fprintf(fd, " %5u",
|
|
|
460672 |
td->td_transferfunction[i][l]);
|
|
|
460672 |
fputc('\n', fd);
|
|
|
460672 |
diff --git a/libtiff/tif_unix.c b/libtiff/tif_unix.c
|
|
|
460672 |
index 0a3a4a8..5c00917 100644
|
|
|
460672 |
--- a/libtiff/tif_unix.c
|
|
|
460672 |
+++ b/libtiff/tif_unix.c
|
|
|
460672 |
@@ -262,6 +262,14 @@ _TIFFmalloc(tmsize_t s)
|
|
|
460672 |
return (malloc((size_t) s));
|
|
|
460672 |
}
|
|
|
460672 |
|
|
|
460672 |
+void* _TIFFcalloc(tmsize_t nmemb, tmsize_t siz)
|
|
|
460672 |
+{
|
|
|
460672 |
+ if( nmemb == 0 || siz == 0 )
|
|
|
460672 |
+ return ((void *) NULL);
|
|
|
460672 |
+
|
|
|
460672 |
+ return calloc((size_t) nmemb, (size_t)siz);
|
|
|
460672 |
+}
|
|
|
460672 |
+
|
|
|
460672 |
void
|
|
|
460672 |
_TIFFfree(void* p)
|
|
|
460672 |
{
|
|
|
460672 |
diff --git a/libtiff/tif_win32.c b/libtiff/tif_win32.c
|
|
|
460672 |
index 4a2466f..ef6be08 100644
|
|
|
460672 |
--- a/libtiff/tif_win32.c
|
|
|
460672 |
+++ b/libtiff/tif_win32.c
|
|
|
460672 |
@@ -334,6 +334,14 @@ _TIFFmalloc(tmsize_t s)
|
|
|
460672 |
return (malloc((size_t) s));
|
|
|
460672 |
}
|
|
|
460672 |
|
|
|
460672 |
+void* _TIFFcalloc(tmsize_t nmemb, tmsize_t siz)
|
|
|
460672 |
+{
|
|
|
460672 |
+ if( nmemb == 0 || siz == 0 )
|
|
|
460672 |
+ return ((void *) NULL);
|
|
|
460672 |
+
|
|
|
460672 |
+ return calloc((size_t) nmemb, (size_t)siz);
|
|
|
460672 |
+}
|
|
|
460672 |
+
|
|
|
460672 |
void
|
|
|
460672 |
_TIFFfree(void* p)
|
|
|
460672 |
{
|
|
|
460672 |
diff --git a/libtiff/tiffio.h b/libtiff/tiffio.h
|
|
|
460672 |
index 038b670..9b39392 100644
|
|
|
460672 |
--- a/libtiff/tiffio.h
|
|
|
460672 |
+++ b/libtiff/tiffio.h
|
|
|
460672 |
@@ -293,6 +293,7 @@ extern TIFFCodec* TIFFGetConfiguredCODECs(void);
|
|
|
460672 |
*/
|
|
|
460672 |
|
|
|
460672 |
extern void* _TIFFmalloc(tmsize_t s);
|
|
|
460672 |
+extern void* _TIFFcalloc(tmsize_t nmemb, tmsize_t siz);
|
|
|
460672 |
extern void* _TIFFrealloc(void* p, tmsize_t s);
|
|
|
460672 |
extern void _TIFFmemset(void* p, int v, tmsize_t c);
|
|
|
460672 |
extern void _TIFFmemcpy(void* d, const void* s, tmsize_t c);
|
|
|
460672 |
--
|
|
|
460672 |
2.17.2
|
|
|
460672 |
|