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