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
|
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.
CVE: CVE-2022-3599 CVE-2022-4645
Upstream-Status: Backport [https://gitlab.com/libtiff/libtiff/-/commit/e813112545942107551433d61afd16ac094ff246.patch]
Signed-off-by: Ross Burton <ross.burton@arm.com>
Signed-off-by: Pawan Badganchi <Pawan.Badganchi@kpit.com>
---
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(-)
diff --git a/libtiff/tif_dir.c b/libtiff/tif_dir.c
index 793e8a79..816f7756 100644
--- a/libtiff/tif_dir.c
+++ b/libtiff/tif_dir.c
@@ -136,32 +136,30 @@ setExtraSamples(TIFF* tif, va_list ap, uint32_t* v)
}
/*
- * 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_t
-checkInkNamesString(TIFF* tif, uint32_t slen, const char* s)
+static uint16_t
+countInkNamesString(TIFF *tif, uint32_t slen, const char *s)
{
- TIFFDirectory* td = &tif->tif_dir;
- uint16_t i = td->td_samplesperpixel;
+ uint16_t 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_t)(cp - s));
+ i++;
+ } while (cp < ep);
+ return (i);
}
bad:
TIFFErrorExt(tif->tif_clientdata, "TIFFSetField",
- "%s: Invalid InkNames value; expecting %"PRIu16" names, found %"PRIu16,
- tif->tif_name,
- td->td_samplesperpixel,
- (uint16_t)(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);
}
@@ -478,13 +476,61 @@ _TIFFVSetField(TIFF* tif, uint32_t tag, va_list ap)
_TIFFsetFloatArray(&td->td_refblackwhite, va_arg(ap, float*), 6);
break;
case TIFFTAG_INKNAMES:
- v = (uint16_t) 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_t) va_arg(ap, uint16_vap);
+ s = va_arg(ap, char*);
+ uint16_t 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_t)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_t)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:
@@ -986,34 +1032,6 @@ _TIFFVGetField(TIFF* tif, uint32_t tag, va_list ap)
if (fip->field_bit == FIELD_CUSTOM) {
standard_tag = 0;
}
-
- if( standard_tag == TIFFTAG_NUMBEROFINKS )
- {
- int i;
- for (i = 0; i < td->td_customValueCount; i++) {
- uint16_t val;
- TIFFTagValue *tv = td->td_customValues + i;
- if (tv->info->field_tag != standard_tag)
- continue;
- if( tv->value == NULL )
- return 0;
- val = *(uint16_t *)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 %"PRIu16,
- val, td->td_samplesperpixel);
- val = td->td_samplesperpixel;
- }
- *va_arg(ap, uint16_t*) = val;
- return 1;
- }
- return 0;
- }
switch (standard_tag) {
case TIFFTAG_SUBFILETYPE:
@@ -1195,6 +1213,9 @@ _TIFFVGetField(TIFF* tif, uint32_t tag, va_list ap)
case TIFFTAG_INKNAMES:
*va_arg(ap, const char**) = td->td_inknames;
break;
+ case TIFFTAG_NUMBEROFINKS:
+ *va_arg(ap, uint16_t *) = td->td_numberofinks;
+ break;
default:
{
int i;
diff --git a/libtiff/tif_dir.h b/libtiff/tif_dir.h
index 09065648..0c251c9e 100644
--- a/libtiff/tif_dir.h
+++ b/libtiff/tif_dir.h
@@ -117,6 +117,7 @@ typedef struct {
/* CMYK parameters */
int td_inknameslen;
char* td_inknames;
+ uint16_t 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 */
diff --git a/libtiff/tif_dirinfo.c b/libtiff/tif_dirinfo.c
index 3371cb5c..3b4bcd33 100644
--- a/libtiff/tif_dirinfo.c
+++ b/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 },
diff --git a/libtiff/tif_dirwrite.c b/libtiff/tif_dirwrite.c
index 6c86fdca..062e4610 100644
--- a/libtiff/tif_dirwrite.c
+++ b/libtiff/tif_dirwrite.c
@@ -626,6 +626,11 @@ TIFFWriteDirectorySec(TIFF* tif, int isimage, int imagedone, uint64_t* pdiroff)
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))
diff --git a/libtiff/tif_print.c b/libtiff/tif_print.c
index 16ce5780..a91b9e7b 100644
--- a/libtiff/tif_print.c
+++ b/libtiff/tif_print.c
@@ -397,6 +397,10 @@ TIFFPrintDirectory(TIFF* tif, FILE* fd, long flags)
}
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) {
--
2.34.1
|
