262 lines
11 KiB
Diff
262 lines
11 KiB
Diff
From 80d781f24346e2ba76e9eedfc943f6013abb2771 Mon Sep 17 00:00:00 2001
|
|
From: Su_Laus <sulau@freenet.de>
|
|
Date: Tue, 30 Aug 2022 16:56:48 +0200
|
|
Subject: [PATCH] (CVE-2022-3599) 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.
|
|
|
|
(cherry picked from commit f00484b9519df933723deb38fff943dc291a793d)
|
|
---
|
|
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 e90f14a0..a4295dc9 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);
|
|
}
|
|
|
|
@@ -475,13 +473,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:
|
|
@@ -915,34 +961,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:
|
|
@@ -1124,6 +1142,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 c30f569b..a7e78aae 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 2fef6d82..1a00edbf 100644
|
|
--- a/libtiff/tif_dirwrite.c
|
|
+++ b/libtiff/tif_dirwrite.c
|
|
@@ -708,6 +708,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 80a9d90f..1ed90e28 100644
|
|
--- a/libtiff/tif_print.c
|
|
+++ b/libtiff/tif_print.c
|
|
@@ -401,6 +401,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) {
|