|
|
- From de5385cd882a5ff0970f63f4d93da0cbc87230c2 Mon Sep 17 00:00:00 2001
- From: =?UTF-8?q?Nikola=20Forr=C3=B3?= <nforro@redhat.com>
- Date: Tue, 17 Apr 2018 18:42:09 +0200
- 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 | 62 +++++++++++++++++++++++++++++++++++++++++++
- libtiff/tif_print.c | 2 +-
- 2 files changed, 63 insertions(+), 1 deletion(-)
-
- diff --git a/libtiff/tif_dirread.c b/libtiff/tif_dirread.c
- index 5e62e81..80aaf8d 100644
- --- a/libtiff/tif_dirread.c
- +++ b/libtiff/tif_dirread.c
- @@ -167,6 +167,7 @@ static int TIFFFetchStripThing(TIFF* tif, TIFFDirEntry* dir, uint32 nstrips, uin
- 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 );
-
- @@ -3506,6 +3507,35 @@ static void TIFFReadDirEntryOutputErr(TIFF* tif, enum TIFFReadDirEntryErr err, c
- }
- }
-
- +/*
- + * 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.
- @@ -3522,6 +3552,7 @@ TIFFReadDirectory(TIFF* tif)
- 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))
- @@ -4026,6 +4057,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 24d4b98..10a588e 100644
- --- a/libtiff/tif_print.c
- +++ b/libtiff/tif_print.c
- @@ -546,7 +546,7 @@ TIFFPrintDirectory(TIFF* tif, FILE* fd, long flags)
- 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);
- --
- 2.17.0
-
|
