Changeset View
Changeset View
Standalone View
Standalone View
files/security/cve-2018-7456.patch
- This file was added.
| 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 | |||||
Copyright © 2015-2021 Solus Project. The Solus logo is Copyright © 2016-2021 Solus Project. All Rights Reserved.