1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268
|
From f00484b9519df933723deb38fff943dc291a793d Mon Sep 17 00:00:00 2001
From: Su_Laus <sulau@freenet.de>
Date: Tue, 30 Aug 2022 16:56:48 +0200
Subject: [PATCH] Revised handling of TIFFTAG_INKNAMES and related
TIFFTAG_NUMBEROFINKS value
In order to solve the buffer overflow issues related to TIFFTAG_INKNAMES and related TIFFTAG_NUMBEROFINKS value, a revised handling of those tags within LibTiff is proposed:
Behaviour for writing:
`NumberOfInks` MUST fit to the number of inks in the `InkNames` string.
`NumberOfInks` is automatically set when `InkNames` is set.
If `NumberOfInks` is different to the number of inks within `InkNames` string, that will be corrected and a warning is issued.
If `NumberOfInks` is not equal to samplesperpixel only a warning will be issued.
Behaviour for reading:
When reading `InkNames` from a TIFF file, the `NumberOfInks` will be set automatically to the number of inks in `InkNames` string.
If `NumberOfInks` is different to the number of inks within `InkNames` string, that will be corrected and a warning is issued.
If `NumberOfInks` is not equal to samplesperpixel only a warning will be issued.
This allows the safe use of the NumberOfInks value to read out the InkNames without buffer overflow
This MR will close the following issues: #149, #150, #152, #168 (to be checked), #250, #269, #398 and #456.
It also fixes the old bug at http://bugzilla.maptools.org/show_bug.cgi?id=2599, for which the limitation of `NumberOfInks = SPP` was introduced, which is in my opinion not necessary and does not solve the general issue.
---
libtiff/tif_dir.c | 119 ++++++++++++++++++++++++-----------------
libtiff/tif_dir.h | 2 +
libtiff/tif_dirinfo.c | 2 +-
libtiff/tif_dirwrite.c | 5 ++
libtiff/tif_print.c | 4 ++
5 files changed, 82 insertions(+), 50 deletions(-)
Index: tiff-4.2.0/libtiff/tif_dir.c
===================================================================
--- tiff-4.2.0.orig/libtiff/tif_dir.c
+++ tiff-4.2.0/libtiff/tif_dir.c
@@ -31,6 +31,8 @@
#include "tiffiop.h"
#include <float.h> /*--: for Rational2Double */
+#include <inttypes.h>
+
/*
* These are used in the backwards compatibility code...
*/
@@ -138,32 +140,30 @@ setExtraSamples(TIFF* tif, va_list ap, u
}
/*
- * Confirm we have "samplesperpixel" ink names separated by \0. Returns
+ * Count ink names separated by \0. Returns
* zero if the ink names are not as expected.
*/
-static uint32
-checkInkNamesString(TIFF* tif, uint32 slen, const char* s)
+static uint16
+countInkNamesString(TIFF *tif, uint32 slen, const char *s)
{
- TIFFDirectory* td = &tif->tif_dir;
- uint16 i = td->td_samplesperpixel;
+ uint16 i = 0;
+ const char *ep = s + slen;
+ const char *cp = s;
if (slen > 0) {
- const char* ep = s+slen;
- const char* cp = s;
- for (; i > 0; i--) {
+ do {
for (; cp < ep && *cp != '\0'; cp++) {}
if (cp >= ep)
goto bad;
cp++; /* skip \0 */
- }
- return ((uint32)(cp-s));
+ i++;
+ } while (cp < ep);
+ return (i);
}
bad:
TIFFErrorExt(tif->tif_clientdata, "TIFFSetField",
- "%s: Invalid InkNames value; expecting %d names, found %d",
- tif->tif_name,
- td->td_samplesperpixel,
- td->td_samplesperpixel-i);
+ "%s: Invalid InkNames value; no NUL at given buffer end location %"PRIu32", after %"PRIu16" ink",
+ tif->tif_name, slen, i);
return (0);
}
@@ -477,13 +477,61 @@ _TIFFVSetField(TIFF* tif, uint32 tag, va
_TIFFsetFloatArray(&td->td_refblackwhite, va_arg(ap, float*), 6);
break;
case TIFFTAG_INKNAMES:
- v = (uint16) va_arg(ap, uint16_vap);
- s = va_arg(ap, char*);
- v = checkInkNamesString(tif, v, s);
- status = v > 0;
- if( v > 0 ) {
- _TIFFsetNString(&td->td_inknames, s, v);
- td->td_inknameslen = v;
+ {
+ v = (uint16) va_arg(ap, uint16_vap);
+ s = va_arg(ap, char*);
+ uint16 ninksinstring;
+ ninksinstring = countInkNamesString(tif, v, s);
+ status = ninksinstring > 0;
+ if(ninksinstring > 0 ) {
+ _TIFFsetNString(&td->td_inknames, s, v);
+ td->td_inknameslen = v;
+ /* Set NumberOfInks to the value ninksinstring */
+ if (TIFFFieldSet(tif, FIELD_NUMBEROFINKS))
+ {
+ if (td->td_numberofinks != ninksinstring) {
+ TIFFErrorExt(tif->tif_clientdata, module,
+ "Warning %s; Tag %s:\n Value %"PRIu16" of NumberOfInks is different from the number of inks %"PRIu16".\n -> NumberOfInks value adapted to %"PRIu16"",
+ tif->tif_name, fip->field_name, td->td_numberofinks, ninksinstring, ninksinstring);
+ td->td_numberofinks = ninksinstring;
+ }
+ } else {
+ td->td_numberofinks = ninksinstring;
+ TIFFSetFieldBit(tif, FIELD_NUMBEROFINKS);
+ }
+ if (TIFFFieldSet(tif, FIELD_SAMPLESPERPIXEL))
+ {
+ if (td->td_numberofinks != td->td_samplesperpixel) {
+ TIFFErrorExt(tif->tif_clientdata, module,
+ "Warning %s; Tag %s:\n Value %"PRIu16" of NumberOfInks is different from the SamplesPerPixel value %"PRIu16"",
+ tif->tif_name, fip->field_name, td->td_numberofinks, td->td_samplesperpixel);
+ }
+ }
+ }
+ }
+ break;
+ case TIFFTAG_NUMBEROFINKS:
+ v = (uint16)va_arg(ap, uint16_vap);
+ /* If InkNames already set also NumberOfInks is set accordingly and should be equal */
+ if (TIFFFieldSet(tif, FIELD_INKNAMES))
+ {
+ if (v != td->td_numberofinks) {
+ TIFFErrorExt(tif->tif_clientdata, module,
+ "Error %s; Tag %s:\n It is not possible to set the value %"PRIu32" for NumberOfInks\n which is different from the number of inks in the InkNames tag (%"PRIu16")",
+ tif->tif_name, fip->field_name, v, td->td_numberofinks);
+ /* Do not set / overwrite number of inks already set by InkNames case accordingly. */
+ status = 0;
+ }
+ } else {
+ td->td_numberofinks = (uint16)v;
+ if (TIFFFieldSet(tif, FIELD_SAMPLESPERPIXEL))
+ {
+ if (td->td_numberofinks != td->td_samplesperpixel) {
+ TIFFErrorExt(tif->tif_clientdata, module,
+ "Warning %s; Tag %s:\n Value %"PRIu32" of NumberOfInks is different from the SamplesPerPixel value %"PRIu16"",
+ tif->tif_name, fip->field_name, v, td->td_samplesperpixel);
+ }
+ }
}
break;
case TIFFTAG_PERSAMPLE:
@@ -910,34 +958,6 @@ _TIFFVGetField(TIFF* tif, uint32 tag, va
standard_tag = 0;
}
- if( standard_tag == TIFFTAG_NUMBEROFINKS )
- {
- int i;
- for (i = 0; i < td->td_customValueCount; i++) {
- uint16 val;
- TIFFTagValue *tv = td->td_customValues + i;
- if (tv->info->field_tag != standard_tag)
- continue;
- if( tv->value == NULL )
- return 0;
- val = *(uint16 *)tv->value;
- /* Truncate to SamplesPerPixel, since the */
- /* setting code for INKNAMES assume that there are SamplesPerPixel */
- /* inknames. */
- /* Fixes http://bugzilla.maptools.org/show_bug.cgi?id=2599 */
- if( val > td->td_samplesperpixel )
- {
- TIFFWarningExt(tif->tif_clientdata,"_TIFFVGetField",
- "Truncating NumberOfInks from %u to %u",
- val, td->td_samplesperpixel);
- val = td->td_samplesperpixel;
- }
- *va_arg(ap, uint16*) = val;
- return 1;
- }
- return 0;
- }
-
switch (standard_tag) {
case TIFFTAG_SUBFILETYPE:
*va_arg(ap, uint32*) = td->td_subfiletype;
@@ -1114,6 +1134,9 @@ _TIFFVGetField(TIFF* tif, uint32 tag, va
case TIFFTAG_INKNAMES:
*va_arg(ap, const char**) = td->td_inknames;
break;
+ case TIFFTAG_NUMBEROFINKS:
+ *va_arg(ap, uint16 *) = td->td_numberofinks;
+ break;
default:
{
int i;
Index: tiff-4.2.0/libtiff/tif_dir.h
===================================================================
--- tiff-4.2.0.orig/libtiff/tif_dir.h
+++ tiff-4.2.0/libtiff/tif_dir.h
@@ -117,6 +117,7 @@ typedef struct {
/* CMYK parameters */
int td_inknameslen;
char* td_inknames;
+ uint16 td_numberofinks; /* number of inks in InkNames string */
int td_customValueCount;
TIFFTagValue *td_customValues;
@@ -174,6 +175,7 @@ typedef struct {
#define FIELD_TRANSFERFUNCTION 44
#define FIELD_INKNAMES 46
#define FIELD_SUBIFD 49
+#define FIELD_NUMBEROFINKS 50
/* FIELD_CUSTOM (see tiffio.h) 65 */
/* end of support for well-known tags; codec-private tags follow */
#define FIELD_CODEC 66 /* base of codec-private tags */
Index: tiff-4.2.0/libtiff/tif_dirinfo.c
===================================================================
--- tiff-4.2.0.orig/libtiff/tif_dirinfo.c
+++ tiff-4.2.0/libtiff/tif_dirinfo.c
@@ -114,7 +114,7 @@ tiffFields[] = {
{ TIFFTAG_SUBIFD, -1, -1, TIFF_IFD8, 0, TIFF_SETGET_C16_IFD8, TIFF_SETGET_UNDEFINED, FIELD_SUBIFD, 1, 1, "SubIFD", (TIFFFieldArray*) &tiffFieldArray },
{ TIFFTAG_INKSET, 1, 1, TIFF_SHORT, 0, TIFF_SETGET_UINT16, TIFF_SETGET_UNDEFINED, FIELD_CUSTOM, 0, 0, "InkSet", NULL },
{ TIFFTAG_INKNAMES, -1, -1, TIFF_ASCII, 0, TIFF_SETGET_C16_ASCII, TIFF_SETGET_UNDEFINED, FIELD_INKNAMES, 1, 1, "InkNames", NULL },
- { TIFFTAG_NUMBEROFINKS, 1, 1, TIFF_SHORT, 0, TIFF_SETGET_UINT16, TIFF_SETGET_UNDEFINED, FIELD_CUSTOM, 1, 0, "NumberOfInks", NULL },
+ { TIFFTAG_NUMBEROFINKS, 1, 1, TIFF_SHORT, 0, TIFF_SETGET_UINT16, TIFF_SETGET_UNDEFINED, FIELD_NUMBEROFINKS, 1, 0, "NumberOfInks", NULL },
{ TIFFTAG_DOTRANGE, 2, 2, TIFF_SHORT, 0, TIFF_SETGET_UINT16_PAIR, TIFF_SETGET_UNDEFINED, FIELD_CUSTOM, 0, 0, "DotRange", NULL },
{ TIFFTAG_TARGETPRINTER, -1, -1, TIFF_ASCII, 0, TIFF_SETGET_ASCII, TIFF_SETGET_UNDEFINED, FIELD_CUSTOM, 1, 0, "TargetPrinter", NULL },
{ TIFFTAG_EXTRASAMPLES, -1, -1, TIFF_SHORT, 0, TIFF_SETGET_C16_UINT16, TIFF_SETGET_UNDEFINED, FIELD_EXTRASAMPLES, 0, 1, "ExtraSamples", NULL },
Index: tiff-4.2.0/libtiff/tif_dirwrite.c
===================================================================
--- tiff-4.2.0.orig/libtiff/tif_dirwrite.c
+++ tiff-4.2.0/libtiff/tif_dirwrite.c
@@ -692,6 +692,11 @@ TIFFWriteDirectorySec(TIFF* tif, int isi
if (!TIFFWriteDirectoryTagAscii(tif,&ndir,dir,TIFFTAG_INKNAMES,tif->tif_dir.td_inknameslen,tif->tif_dir.td_inknames))
goto bad;
}
+ if (TIFFFieldSet(tif, FIELD_NUMBEROFINKS))
+ {
+ if (!TIFFWriteDirectoryTagShort(tif, &ndir, dir, TIFFTAG_NUMBEROFINKS, tif->tif_dir.td_numberofinks))
+ goto bad;
+ }
if (TIFFFieldSet(tif,FIELD_SUBIFD))
{
if (!TIFFWriteDirectoryTagSubifd(tif,&ndir,dir))
Index: tiff-4.2.0/libtiff/tif_print.c
===================================================================
--- tiff-4.2.0.orig/libtiff/tif_print.c
+++ tiff-4.2.0/libtiff/tif_print.c
@@ -402,6 +402,10 @@ TIFFPrintDirectory(TIFF* tif, FILE* fd,
}
fputs("\n", fd);
}
+ if (TIFFFieldSet(tif, FIELD_NUMBEROFINKS)) {
+ fprintf(fd, " NumberOfInks: %d\n",
+ td->td_numberofinks);
+ }
if (TIFFFieldSet(tif,FIELD_THRESHHOLDING)) {
fprintf(fd, " Thresholding: ");
switch (td->td_threshholding) {
|