Much of the original content of this patch has been removed in favor of the more robust solution represented by libtiff-unknown-tags.patch. diff -Naur tiff-3.8.2.orig/libtiff/tif_dir.c tiff-3.8.2/libtiff/tif_dir.c --- tiff-3.8.2.orig/libtiff/tif_dir.c 2006-03-21 11:42:50.000000000 -0500 +++ tiff-3.8.2/libtiff/tif_dir.c 2010-06-16 21:11:51.000000000 -0400 @@ -468,7 +468,7 @@ if (fip->field_type == TIFF_ASCII) _TIFFsetString((char **)&tv->value, va_arg(ap, char *)); else { - tv->value = _TIFFmalloc(tv_size * tv->count); + tv->value = _TIFFCheckMalloc(tif, tv_size, tv->count, "Tag Value"); if (!tv->value) { status = 0; goto end; diff -Naur tiff-3.8.2.orig/libtiff/tif_dirread.c tiff-3.8.2/libtiff/tif_dirread.c --- tiff-3.8.2.orig/libtiff/tif_dirread.c 2006-03-21 11:42:50.000000000 -0500 +++ tiff-3.8.2/libtiff/tif_dirread.c 2010-06-16 21:11:51.000000000 -0400 @@ -29,6 +29,9 @@ * * Directory Read Support Routines. */ + +#include <limits.h> + #include "tiffiop.h" #define IGNORE 0 /* tag placeholder used below */ @@ -147,13 +150,20 @@ } else { toff_t off = tif->tif_diroff; - if (off + sizeof (uint16) > tif->tif_size) { - TIFFErrorExt(tif->tif_clientdata, module, - "%s: Can not read TIFF directory count", - tif->tif_name); - return (0); + /* + * Check for integer overflow when validating the dir_off, otherwise + * a very high offset may cause an OOB read and crash the client. + * -- taviso@google.com, 14 Jun 2006. + */ + if (off + sizeof (uint16) > tif->tif_size || + off > (UINT_MAX - sizeof(uint16))) { + TIFFErrorExt(tif->tif_clientdata, module, + "%s: Can not read TIFF directory count", + tif->tif_name); + return (0); } else - _TIFFmemcpy(&dircount, tif->tif_base + off, sizeof (uint16)); + _TIFFmemcpy(&dircount, tif->tif_base + off, + sizeof (uint16)); off += sizeof (uint16); if (tif->tif_flags & TIFF_SWAB) TIFFSwabShort(&dircount); @@ -870,7 +880,13 @@ register TIFFDirEntry *dp; register TIFFDirectory *td = &tif->tif_dir; - uint16 i; + + /* i is used to iterate over td->td_nstrips, so must be + * at least the same width. + * -- taviso@google.com 15 Jun 2006 + */ + + uint32 i; if (td->td_stripbytecount) _TIFFfree(td->td_stripbytecount); @@ -1159,6 +1175,20 @@ static int TIFFFetchShortPair(TIFF* tif, TIFFDirEntry* dir) { + /* + * Prevent overflowing the v stack arrays below by performing a sanity + * check on tdir_count, this should never be greater than two. + * -- taviso@google.com 14 Jun 2006. + */ + if (dir->tdir_count > 2) { + const TIFFFieldInfo* fip = _TIFFFieldWithTag(tif, dir->tdir_tag); + TIFFWarningExt(tif->tif_clientdata, tif->tif_name, + "unexpected count for field \"%s\", %lu, expected 2; ignored.", + fip ? fip->field_name : "Unknown", + dir->tdir_count); + return 0; + } + switch (dir->tdir_type) { case TIFF_BYTE: case TIFF_SBYTE: diff -Naur tiff-3.8.2.orig/libtiff/tif_jpeg.c tiff-3.8.2/libtiff/tif_jpeg.c --- tiff-3.8.2.orig/libtiff/tif_jpeg.c 2006-03-21 11:42:50.000000000 -0500 +++ tiff-3.8.2/libtiff/tif_jpeg.c 2010-06-16 21:11:51.000000000 -0400 @@ -722,15 +722,31 @@ segment_width = TIFFhowmany(segment_width, sp->h_sampling); segment_height = TIFFhowmany(segment_height, sp->v_sampling); } - if (sp->cinfo.d.image_width != segment_width || - sp->cinfo.d.image_height != segment_height) { + if (sp->cinfo.d.image_width < segment_width || + sp->cinfo.d.image_height < segment_height) { TIFFWarningExt(tif->tif_clientdata, module, "Improper JPEG strip/tile size, expected %dx%d, got %dx%d", segment_width, segment_height, sp->cinfo.d.image_width, sp->cinfo.d.image_height); + } + + if (sp->cinfo.d.image_width > segment_width || + sp->cinfo.d.image_height > segment_height) { + /* + * This case could be dangerous, if the strip or tile size has been + * reported as less than the amount of data jpeg will return, some + * potential security issues arise. Catch this case and error out. + * -- taviso@google.com 14 Jun 2006 + */ + TIFFErrorExt(tif->tif_clientdata, module, + "JPEG strip/tile size exceeds expected dimensions," + "expected %dx%d, got %dx%d", segment_width, segment_height, + sp->cinfo.d.image_width, sp->cinfo.d.image_height); + return (0); } + if (sp->cinfo.d.num_components != (td->td_planarconfig == PLANARCONFIG_CONTIG ? td->td_samplesperpixel : 1)) { @@ -761,6 +777,22 @@ sp->cinfo.d.comp_info[0].v_samp_factor, sp->h_sampling, sp->v_sampling); + /* + * There are potential security issues here for decoders that + * have already allocated buffers based on the expected sampling + * factors. Lets check the sampling factors dont exceed what + * we were expecting. + * -- taviso@google.com 14 June 2006 + */ + if (sp->cinfo.d.comp_info[0].h_samp_factor > sp->h_sampling || + sp->cinfo.d.comp_info[0].v_samp_factor > sp->v_sampling) { + TIFFErrorExt(tif->tif_clientdata, module, + "Cannot honour JPEG sampling factors that" + " exceed those specified."); + return (0); + } + + /* * XXX: Files written by the Intergraph software * has different sampling factors stored in the @@ -1521,15 +1553,18 @@ { JPEGState *sp = JState(tif); - assert(sp != 0); + /* assert(sp != 0); */ tif->tif_tagmethods.vgetfield = sp->vgetparent; tif->tif_tagmethods.vsetfield = sp->vsetparent; - if( sp->cinfo_initialized ) - TIFFjpeg_destroy(sp); /* release libjpeg resources */ - if (sp->jpegtables) /* tag value */ - _TIFFfree(sp->jpegtables); + if (sp != NULL) { + if( sp->cinfo_initialized ) + TIFFjpeg_destroy(sp); /* release libjpeg resources */ + if (sp->jpegtables) /* tag value */ + _TIFFfree(sp->jpegtables); + } + _TIFFfree(tif->tif_data); /* release local state */ tif->tif_data = NULL; @@ -1726,7 +1761,11 @@ { JPEGState* sp = JState(tif); - assert(sp != NULL); + /* assert(sp != NULL); */ + if (sp == NULL) { + TIFFWarningExt(tif->tif_clientdata, "JPEGPrintDir", "Unknown JPEGState"); + return; + } (void) flags; if (TIFFFieldSet(tif,FIELD_JPEGTABLES)) diff -Naur tiff-3.8.2.orig/libtiff/tif_next.c tiff-3.8.2/libtiff/tif_next.c --- tiff-3.8.2.orig/libtiff/tif_next.c 2005-12-21 07:33:56.000000000 -0500 +++ tiff-3.8.2/libtiff/tif_next.c 2010-06-16 21:11:51.000000000 -0400 @@ -105,11 +105,16 @@ * as codes of the form <color><npixels> * until we've filled the scanline. */ + /* + * Ensure the run does not exceed the scanline + * bounds, potentially resulting in a security issue. + * -- taviso@google.com 14 Jun 2006. + */ op = row; for (;;) { grey = (n>>6) & 0x3; n &= 0x3f; - while (n-- > 0) + while (n-- > 0 && npixels < imagewidth) SETPIXEL(op, grey); if (npixels >= (int) imagewidth) break; diff -Naur tiff-3.8.2.orig/libtiff/tif_pixarlog.c tiff-3.8.2/libtiff/tif_pixarlog.c --- tiff-3.8.2.orig/libtiff/tif_pixarlog.c 2006-03-21 11:42:50.000000000 -0500 +++ tiff-3.8.2/libtiff/tif_pixarlog.c 2010-06-16 21:11:51.000000000 -0400 @@ -768,7 +768,19 @@ if (tif->tif_flags & TIFF_SWAB) TIFFSwabArrayOfShort(up, nsamples); - for (i = 0; i < nsamples; i += llen, up += llen) { + /* + * if llen is not an exact multiple of nsamples, the decode operation + * may overflow the output buffer, so truncate it enough to prevent that + * but still salvage as much data as possible. + * -- taviso@google.com 14th June 2006 + */ + if (nsamples % llen) + TIFFWarningExt(tif->tif_clientdata, module, + "%s: stride %lu is not a multiple of sample count, " + "%lu, data truncated.", tif->tif_name, llen, nsamples); + + + for (i = 0; i < nsamples - (nsamples % llen); i += llen, up += llen) { switch (sp->user_datafmt) { case PIXARLOGDATAFMT_FLOAT: horizontalAccumulateF(up, llen, sp->stride, diff -Naur tiff-3.8.2.orig/libtiff/tif_read.c tiff-3.8.2/libtiff/tif_read.c --- tiff-3.8.2.orig/libtiff/tif_read.c 2005-12-21 07:33:56.000000000 -0500 +++ tiff-3.8.2/libtiff/tif_read.c 2010-06-16 21:11:51.000000000 -0400 @@ -31,6 +31,8 @@ #include "tiffiop.h" #include <stdio.h> +#include <limits.h> + int TIFFFillStrip(TIFF*, tstrip_t); int TIFFFillTile(TIFF*, ttile_t); static int TIFFStartStrip(TIFF*, tstrip_t); @@ -272,7 +274,13 @@ if ((tif->tif_flags & TIFF_MYBUFFER) && tif->tif_rawdata) _TIFFfree(tif->tif_rawdata); tif->tif_flags &= ~TIFF_MYBUFFER; - if ( td->td_stripoffset[strip] + bytecount > tif->tif_size) { + /* + * This sanity check could potentially overflow, causing an OOB read. + * verify that offset + bytecount is > offset. + * -- taviso@google.com 14 Jun 2006 + */ + if ( td->td_stripoffset[strip] + bytecount > tif->tif_size || + bytecount > (UINT_MAX - td->td_stripoffset[strip])) { /* * This error message might seem strange, but it's * what would happen if a read were done instead. @@ -470,7 +478,13 @@ if ((tif->tif_flags & TIFF_MYBUFFER) && tif->tif_rawdata) _TIFFfree(tif->tif_rawdata); tif->tif_flags &= ~TIFF_MYBUFFER; - if ( td->td_stripoffset[tile] + bytecount > tif->tif_size) { + /* + * We must check this calculation doesnt overflow, potentially + * causing an OOB read. + * -- taviso@google.com 15 Jun 2006 + */ + if (td->td_stripoffset[tile] + bytecount > tif->tif_size || + bytecount > (UINT_MAX - td->td_stripoffset[tile])) { tif->tif_curtile = NOTILE; return (0); }