CVE: CVE-2022-3599
Upstream-Status: Backport
Signed-off-by: Ross Burton <ross.burton@arm.com>

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(-)

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