| From be4c85b16e8801a16eec25e80eb9f3dd6a96731b 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 | 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 6baa7b3..af5b84a 100644 |
| |
| |
| @@ -165,6 +165,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 ); |
| |
| @@ -3505,6 +3506,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. |
| */ |
| @@ -3520,6 +3550,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)) |
| @@ -4024,6 +4055,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 8deceb2..1d86adb 100644 |
| |
| |
| @@ -544,7 +544,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); |
| -- |
| libgit2 0.27.0 |
| |