Brad Bishop | d5ae7d9 | 2018-06-14 09:52:03 -0700 | [diff] [blame] | 1 | From be4c85b16e8801a16eec25e80eb9f3dd6a96731b Mon Sep 17 00:00:00 2001 |
| 2 | From: Hugo Lefeuvre <hle@debian.org> |
| 3 | Date: Sun, 8 Apr 2018 14:07:08 -0400 |
| 4 | Subject: [PATCH] Fix NULL pointer dereference in TIFFPrintDirectory |
| 5 | |
| 6 | The TIFFPrintDirectory function relies on the following assumptions, |
| 7 | supposed to be guaranteed by the specification: |
| 8 | |
| 9 | (a) A Transfer Function field is only present if the TIFF file has |
| 10 | photometric type < 3. |
| 11 | |
| 12 | (b) If SamplesPerPixel > Color Channels, then the ExtraSamples field |
| 13 | has count SamplesPerPixel - (Color Channels) and contains |
| 14 | information about supplementary channels. |
| 15 | |
| 16 | While respect of (a) and (b) are essential for the well functioning of |
| 17 | TIFFPrintDirectory, no checks are realized neither by the callee nor |
| 18 | by TIFFPrintDirectory itself. Hence, following scenarios might happen |
| 19 | and trigger the NULL pointer dereference: |
| 20 | |
| 21 | (1) TIFF File of photometric type 4 or more has illegal Transfer |
| 22 | Function field. |
| 23 | |
| 24 | (2) TIFF File has photometric type 3 or less and defines a |
| 25 | SamplesPerPixel field such that SamplesPerPixel > Color Channels |
| 26 | without defining all extra samples in the ExtraSamples fields. |
| 27 | |
| 28 | In this patch, we address both issues with respect of the following |
| 29 | principles: |
| 30 | |
| 31 | (A) In the case of (1), the defined transfer table should be printed |
| 32 | safely even if it isn't 'legal'. This allows us to avoid expensive |
| 33 | checks in TIFFPrintDirectory. Also, it is quite possible that |
| 34 | an alternative photometric type would be developed (not part of the |
| 35 | standard) and would allow definition of Transfer Table. We want |
| 36 | libtiff to be able to handle this scenario out of the box. |
| 37 | |
| 38 | (B) In the case of (2), the transfer table should be printed at its |
| 39 | right size, that is if TIFF file has photometric type Palette |
| 40 | then the transfer table should have one row and not three, even |
| 41 | if two extra samples are declared. |
| 42 | |
| 43 | In order to fulfill (A) we simply add a new 'i < 3' end condition to |
| 44 | the broken TIFFPrintDirectory loop. This makes sure that in any case |
| 45 | where (b) would be respected but not (a), everything stays fine. |
| 46 | |
| 47 | (B) is fulfilled by the loop condition |
| 48 | 'i < td->td_samplesperpixel - td->td_extrasamples'. This is enough as |
| 49 | long as (b) is respected. |
| 50 | |
| 51 | Naturally, we also make sure (b) is respected. This is done in the |
| 52 | TIFFReadDirectory function by making sure any non-color channel is |
| 53 | counted in ExtraSamples. |
| 54 | |
| 55 | This commit addresses CVE-2018-7456. |
| 56 | |
| 57 | --- |
| 58 | CVE: CVE-2018-7456 |
| 59 | |
| 60 | Upstream-Status: Backport [gitlab.com/libtiff/libtiff/commit/be4c85b...] |
| 61 | |
| 62 | Signed-off-by: Joe Slater <joe.slater@windriver.com> |
| 63 | |
| 64 | --- |
| 65 | libtiff/tif_dirread.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++ |
| 66 | libtiff/tif_print.c | 2 +- |
| 67 | 2 files changed, 63 insertions(+), 1 deletion(-) |
| 68 | |
| 69 | diff --git a/libtiff/tif_dirread.c b/libtiff/tif_dirread.c |
| 70 | index 6baa7b3..af5b84a 100644 |
| 71 | --- a/libtiff/tif_dirread.c |
| 72 | +++ b/libtiff/tif_dirread.c |
| 73 | @@ -165,6 +165,7 @@ static int TIFFFetchStripThing(TIFF* tif, TIFFDirEntry* dir, uint32 nstrips, uin |
| 74 | static int TIFFFetchSubjectDistance(TIFF*, TIFFDirEntry*); |
| 75 | static void ChopUpSingleUncompressedStrip(TIFF*); |
| 76 | static uint64 TIFFReadUInt64(const uint8 *value); |
| 77 | +static int _TIFFGetMaxColorChannels(uint16 photometric); |
| 78 | |
| 79 | static int _TIFFFillStrilesInternal( TIFF *tif, int loadStripByteCount ); |
| 80 | |
| 81 | @@ -3505,6 +3506,35 @@ static void TIFFReadDirEntryOutputErr(TIFF* tif, enum TIFFReadDirEntryErr err, c |
| 82 | } |
| 83 | |
| 84 | /* |
| 85 | + * Return the maximum number of color channels specified for a given photometric |
| 86 | + * type. 0 is returned if photometric type isn't supported or no default value |
| 87 | + * is defined by the specification. |
| 88 | + */ |
| 89 | +static int _TIFFGetMaxColorChannels( uint16 photometric ) |
| 90 | +{ |
| 91 | + switch (photometric) { |
| 92 | + case PHOTOMETRIC_PALETTE: |
| 93 | + case PHOTOMETRIC_MINISWHITE: |
| 94 | + case PHOTOMETRIC_MINISBLACK: |
| 95 | + return 1; |
| 96 | + case PHOTOMETRIC_YCBCR: |
| 97 | + case PHOTOMETRIC_RGB: |
| 98 | + case PHOTOMETRIC_CIELAB: |
| 99 | + return 3; |
| 100 | + case PHOTOMETRIC_SEPARATED: |
| 101 | + case PHOTOMETRIC_MASK: |
| 102 | + return 4; |
| 103 | + case PHOTOMETRIC_LOGL: |
| 104 | + case PHOTOMETRIC_LOGLUV: |
| 105 | + case PHOTOMETRIC_CFA: |
| 106 | + case PHOTOMETRIC_ITULAB: |
| 107 | + case PHOTOMETRIC_ICCLAB: |
| 108 | + default: |
| 109 | + return 0; |
| 110 | + } |
| 111 | +} |
| 112 | + |
| 113 | +/* |
| 114 | * Read the next TIFF directory from a file and convert it to the internal |
| 115 | * format. We read directories sequentially. |
| 116 | */ |
| 117 | @@ -3520,6 +3550,7 @@ TIFFReadDirectory(TIFF* tif) |
| 118 | uint32 fii=FAILED_FII; |
| 119 | toff_t nextdiroff; |
| 120 | int bitspersample_read = FALSE; |
| 121 | + int color_channels; |
| 122 | |
| 123 | tif->tif_diroff=tif->tif_nextdiroff; |
| 124 | if (!TIFFCheckDirOffset(tif,tif->tif_nextdiroff)) |
| 125 | @@ -4024,6 +4055,37 @@ TIFFReadDirectory(TIFF* tif) |
| 126 | } |
| 127 | } |
| 128 | } |
| 129 | + |
| 130 | + /* |
| 131 | + * Make sure all non-color channels are extrasamples. |
| 132 | + * If it's not the case, define them as such. |
| 133 | + */ |
| 134 | + color_channels = _TIFFGetMaxColorChannels(tif->tif_dir.td_photometric); |
| 135 | + if (color_channels && tif->tif_dir.td_samplesperpixel - tif->tif_dir.td_extrasamples > color_channels) { |
| 136 | + uint16 old_extrasamples; |
| 137 | + uint16 *new_sampleinfo; |
| 138 | + |
| 139 | + TIFFWarningExt(tif->tif_clientdata,module, "Sum of Photometric type-related " |
| 140 | + "color channels and ExtraSamples doesn't match SamplesPerPixel. " |
| 141 | + "Defining non-color channels as ExtraSamples."); |
| 142 | + |
| 143 | + old_extrasamples = tif->tif_dir.td_extrasamples; |
| 144 | + tif->tif_dir.td_extrasamples = (tif->tif_dir.td_samplesperpixel - color_channels); |
| 145 | + |
| 146 | + // sampleinfo should contain information relative to these new extra samples |
| 147 | + new_sampleinfo = (uint16*) _TIFFcalloc(tif->tif_dir.td_extrasamples, sizeof(uint16)); |
| 148 | + if (!new_sampleinfo) { |
| 149 | + TIFFErrorExt(tif->tif_clientdata, module, "Failed to allocate memory for " |
| 150 | + "temporary new sampleinfo array (%d 16 bit elements)", |
| 151 | + tif->tif_dir.td_extrasamples); |
| 152 | + goto bad; |
| 153 | + } |
| 154 | + |
| 155 | + memcpy(new_sampleinfo, tif->tif_dir.td_sampleinfo, old_extrasamples * sizeof(uint16)); |
| 156 | + _TIFFsetShortArray(&tif->tif_dir.td_sampleinfo, new_sampleinfo, tif->tif_dir.td_extrasamples); |
| 157 | + _TIFFfree(new_sampleinfo); |
| 158 | + } |
| 159 | + |
| 160 | /* |
| 161 | * Verify Palette image has a Colormap. |
| 162 | */ |
| 163 | diff --git a/libtiff/tif_print.c b/libtiff/tif_print.c |
| 164 | index 8deceb2..1d86adb 100644 |
| 165 | --- a/libtiff/tif_print.c |
| 166 | +++ b/libtiff/tif_print.c |
| 167 | @@ -544,7 +544,7 @@ TIFFPrintDirectory(TIFF* tif, FILE* fd, long flags) |
| 168 | uint16 i; |
| 169 | fprintf(fd, " %2ld: %5u", |
| 170 | l, td->td_transferfunction[0][l]); |
| 171 | - for (i = 1; i < td->td_samplesperpixel; i++) |
| 172 | + for (i = 1; i < td->td_samplesperpixel - td->td_extrasamples && i < 3; i++) |
| 173 | fprintf(fd, " %5u", |
| 174 | td->td_transferfunction[i][l]); |
| 175 | fputc('\n', fd); |
| 176 | -- |
| 177 | 1.7.9.5 |
| 178 | |