From 6579f23f3019d8aa7ef0cd856c03d1497add85be Mon Sep 17 00:00:00 2001 From: Hugo Lefeuvre Date: Wed, 21 Nov 2018 18:50:34 +0100 Subject: [PATCH] tif_dir: unset transferfunction field if necessary The number of entries in the transfer table is determined as following: (td->td_samplesperpixel - td->td_extrasamples) > 1 ? 3 : 1 This means that whenever td->td_samplesperpixel or td->td_extrasamples are modified we also need to make sure that the number of required entries in the transfer table didn't change. If it changed and the number of entries is higher than before we should invalidate the transfer table field and free previously allocated values. In the other case there's nothing to do, additional tf entries won't harm and properly written code will just ignore them since spp - es < 1. For instance this situation might happen when reading an OJPEG compressed image with missing SamplesPerPixel tag. In this case the SamplesPerPixel field might be updated after setting the transfer table. see http://bugzilla.maptools.org/show_bug.cgi?id=2500 This commit addresses CVE-2018-19210. --- libtiff/tif_dir.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/libtiff/tif_dir.c b/libtiff/tif_dir.c index 6f0b487..028ea54 100644 --- a/libtiff/tif_dir.c +++ b/libtiff/tif_dir.c @@ -88,13 +88,15 @@ setDoubleArrayOneValue(double** vpp, double value, size_t nmemb) * Install extra samples information. */ static int -setExtraSamples(TIFFDirectory* td, va_list ap, uint32* v) +setExtraSamples(TIFF* tif, va_list ap, uint32* v) { /* XXX: Unassociated alpha data == 999 is a known Corel Draw bug, see below */ #define EXTRASAMPLE_COREL_UNASSALPHA 999 uint16* va; uint32 i; + TIFFDirectory* td = &tif->tif_dir; + static const char module[] = "setExtraSamples"; *v = (uint16) va_arg(ap, uint16_vap); if ((uint16) *v > td->td_samplesperpixel) @@ -116,6 +118,18 @@ setExtraSamples(TIFFDirectory* td, va_list ap, uint32* v) return 0; } } + + if ( td->td_transferfunction[0] != NULL && (td->td_samplesperpixel - *v > 1) && + !(td->td_samplesperpixel - td->td_extrasamples > 1)) + { + TIFFWarningExt(tif->tif_clientdata,module, + "ExtraSamples tag value is changing, " + "but TransferFunction was read with a different value. Cancelling it"); + TIFFClrFieldBit(tif,FIELD_TRANSFERFUNCTION); + _TIFFfree(td->td_transferfunction[0]); + td->td_transferfunction[0] = NULL; + } + td->td_extrasamples = (uint16) *v; _TIFFsetShortArray(&td->td_sampleinfo, va, td->td_extrasamples); return 1; @@ -285,6 +299,18 @@ _TIFFVSetField(TIFF* tif, uint32 tag, va_list ap) _TIFFfree(td->td_smaxsamplevalue); td->td_smaxsamplevalue = NULL; } + /* Test if 3 transfer functions instead of just one are now needed + See http://bugzilla.maptools.org/show_bug.cgi?id=2820 */ + if( td->td_transferfunction[0] != NULL && (v - td->td_extrasamples > 1) && + !(td->td_samplesperpixel - td->td_extrasamples > 1)) + { + TIFFWarningExt(tif->tif_clientdata,module, + "SamplesPerPixel tag value is changing, " + "but TransferFunction was read with a different value. Cancelling it"); + TIFFClrFieldBit(tif,FIELD_TRANSFERFUNCTION); + _TIFFfree(td->td_transferfunction[0]); + td->td_transferfunction[0] = NULL; + } } td->td_samplesperpixel = (uint16) v; break; @@ -361,7 +387,7 @@ _TIFFVSetField(TIFF* tif, uint32 tag, va_list ap) _TIFFsetShortArray(&td->td_colormap[2], va_arg(ap, uint16*), v32); break; case TIFFTAG_EXTRASAMPLES: - if (!setExtraSamples(td, ap, &v)) + if (!setExtraSamples(tif, ap, &v)) goto badvalue; break; case TIFFTAG_MATTEING: -- 2.21.0