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