2014.12.21 15:35 "[Tiff] For review", by Even Rouault
Hi,
In case someone wants to double-check, after painful investigations, I've committed the following changes to avoid crashes/Valgrind errors on a set of fuzzed images that Bob provided to me. I'm not too sure about the stuff related to LOGLUV/CIELAB/ITULAB (changes in libtiff/tif_getimage.c and tools/tiff2pdf.c), so review would be appreciated.
My feeling is that TIFFReadDirectory() is much too tolerant. It can return OK for really corrupted images, and this opens the door to a lot of potential errors in user code/utilities/other parts of the lib that must do later sanity checks, and hardly anybody is good at that. For example, my fix related to ColorMap is really due to accepting images with not increasingly sorted tags. If we had error out before, that issue would have never happened.
Idem for LOGLUV/CIELAB/ITULAB, if those colorspaces have indeed the restrictions I guessed on BitsPerSample/SamplesPerPixel, I feel we should reject them at TIFFReadDirectory() stage, rather than trying to process them in tiff2pdf or TIFFRGBAImageOK().
Even
2014-12-21 Even Rouault <even.rouault@spatialys.com>
Fix various crasher bugs on fuzzed images.
* libtiff/tif_dir.c: TIFFSetField(): refuse to set negative values for
TIFFTAG_XRESOLUTION and TIFFTAG_YRESOLUTION that cause asserts when
writing
the directory
* libtiff/tif_dirread.c: TIFFReadDirectory(): refuse to read ColorMap or
TransferFunction if BitsPerSample has not yet been read, otherwise
reading
it later will cause user code to crash if BitsPerSample > 1
* libtiff/tif_getimage.c: TIFFRGBAImageOK(): return FALSE if LOGLUV with
SamplesPerPixel != 3, or if CIELAB with SamplesPerPixel != 3 or
BitsPerSample != 8
* libtiff/tif_next.c: in the "run mode", use tilewidth for tiled images
instead of imagewidth to avoid crash
* tools/bmp2tiff.c: fix crash due to int overflow related to input BMP
dimensions
* tools/tiff2pdf.c: fix crash due to invalid tile count (should likely be
checked by
libtiff too). Detect invalid settings of BitsPerSample/SamplesPerPixel for
CIELAB / ITULAB
* tools/tiffcrop.c: fix crash due to invalid TileWidth/TileHeight
* tools/tiffdump.c: fix crash due to overflow of entry count.
Index: libtiff/tif_dir.c
===================================================================
RCS file: /cvs/maptools/cvsroot/libtiff/libtiff/tif_dir.c,v
retrieving revision 1.117
diff -r1.117 tif_dir.c
162a163
> double dblval;
287c288,291
< td->td_xresolution = (float) va_arg(ap, double);
---
> dblval = va_arg(ap, double);
> if( dblval < 0 )
> goto badvaluedouble;
> td->td_xresolution = (float) dblval;
290c294,297
< td->td_yresolution = (float) va_arg(ap, double);
---
> dblval = va_arg(ap, double);
> if( dblval < 0 )
> goto badvaluedouble;
> td->td_yresolution = (float) dblval;
696a704,713
> badvaluedouble:
> {
> const TIFFField* fip=TIFFFieldWithTag(tif,tag);
> TIFFErrorExt(tif->tif_clientdata, module,
> "%s: Bad value %f for \"%s\" tag",
> tif->tif_name, dblval,
> fip ? fip->field_name : "Unknown");
> va_end(ap);
> }
> return (0);
Index: libtiff/tif_dirread.c
===================================================================
RCS file: /cvs/maptools/cvsroot/libtiff/libtiff/tif_dirread.c,v retrieving revision 1.180
diff -r1.180 tif_dirread.c
3432a3433,3434
> int bitspersample_read = FALSE;
>
3708a3711,3712
> if( dp->tdir_tag == TIFFTAG_BITSPERSAMPLE )
> bitspersample_read = TRUE;
3765a3770,3782
> /* It would be dangerous to instanciate those tag values
*/
> /* since if td_bitspersample has not yet been read (due
to */
> /* unordered tags), it could be read afterwards with a
*/
> /* values greater than the default one (1), which may
cause */
> /* crashes in user code */
> if( !bitspersample_read )
> {
> fip = TIFFFieldWithTag(tif,dp->tdir_tag);
> TIFFWarningExt(tif->tif_clientdata,module,
> "Ignoring %s since BitsPerSample tag
not found",
> fip? fip->field_name: "unknown
tagname");
> continue;
> }
Index: libtiff/tif_getimage.c
=================================================================== RCS file: /cvs/maptools/cvsroot/libtiff/libtiff/tif_getimage.c,v retrieving revision 1.82
diff -r1.82 tif_getimage.c
184a185,191
> if( td->td_samplesperpixel != 3 )
> {
> sprintf(emsg,
> "Sorry, can not handle image with %s=%d",
> "Samples/pixel", td->td_samplesperpixel);
> return 0;
> }
186a194,201
> if( td->td_samplesperpixel != 3 || td->td_bitspersample != 8 )
> {
> sprintf(emsg,
> "Sorry, can not handle image with %s=%d and %s=%d",
> "Samples/pixel", td->td_samplesperpixel,
> "Bits/sample", td->td_bitspersample);
> return 0;
> }
Index: libtiff/tif_next.c
===================================================================
RCS file: /cvs/maptools/cvsroot/libtiff/libtiff/tif_next.c,v
retrieving revision 1.13
diff -r1.13 tif_next.c
104a105,106
> if( isTiled(tif) )
> imagewidth = tif->tif_dir.td_tilewidth;
Index: tools/bmp2tiff.c
===================================================================
RCS file: /cvs/maptools/cvsroot/libtiff/tools/bmp2tiff.c,v
retrieving revision 1.23
diff -r1.23 bmp2tiff.c
405a406,412
> if( width <= 0 || length <= 0 )
> {
> TIFFError(infilename,
> "Invalid dimensions of BMP file" );
> close(fd);
> return -1;
> }
595a603,610
> /* Detect int overflow */
> if( uncompr_size / width != length )
> {
> TIFFError(infilename,
> "Invalid dimensions of BMP file" );
> close(fd);
> return -1;
> }
Index: tools/tiff2pdf.c
===================================================================
RCS file: /cvs/maptools/cvsroot/libtiff/tools/tiff2pdf.c,v retrieving revision 1.77
diff -r1.77 tiff2pdf.c
1169a1170,1178
> if( (t2p->tiff_tiles[i].tiles_tilecount % xuint16) != 0 )
> {
> TIFFError(
> TIFF2PDF_MODULE,
> "Invalid tile count, %s",
> TIFFFileName(input));
> t2p->t2p_error = T2P_ERR_ERROR;
> return;
> }
1554a1564,1579
> if( t2p->tiff_samplesperpixel != 3){
> TIFFError(
> TIFF2PDF_MODULE,
> "Unsupported samplesperpixel = %d for CIELAB",
> t2p->tiff_samplesperpixel);
> t2p->t2p_error = T2P_ERR_ERROR;
> return;
> }
> if( t2p->tiff_bitspersample != 8){
> TIFFError(
> TIFF2PDF_MODULE,
> "Invalid bitspersample = %d for CIELAB",
> t2p->tiff_bitspersample);
> t2p->t2p_error = T2P_ERR_ERROR;
> return;
> }
1569a1595,1610
> if( t2p->tiff_samplesperpixel != 3){
> TIFFError(
> TIFF2PDF_MODULE,
> "Unsupported samplesperpixel = %d for ITULAB",
> t2p->tiff_samplesperpixel);
> t2p->t2p_error = T2P_ERR_ERROR;
> return;
> }
> if( t2p->tiff_bitspersample != 8){
> TIFFError(
> TIFF2PDF_MODULE,
> "Invalid bitspersample = %d for ITULAB",
> t2p->tiff_bitspersample);
> t2p->t2p_error = T2P_ERR_ERROR;
> return;
> }
Index: tools/tiffcrop.c
===================================================================
RCS file: /cvs/maptools/cvsroot/libtiff/tools/tiffcrop.c,v
retrieving revision 1.23
diff -r1.23 tiffcrop.c
1208,1210c1208,1211
< TIFFGetField(out, TIFFTAG_TILELENGTH, &tl);
< TIFFGetField(out, TIFFTAG_TILEWIDTH, &tw);
< TIFFGetField(out, TIFFTAG_BITSPERSAMPLE, &bps);
---
> if( !TIFFGetField(out, TIFFTAG_TILELENGTH, &tl) ||
> !TIFFGetField(out, TIFFTAG_TILEWIDTH, &tw) ||
> !TIFFGetField(out, TIFFTAG_BITSPERSAMPLE, &bps) )
> return 1;
Index: tools/tiffdump.c
===================================================================
RCS file: /cvs/maptools/cvsroot/libtiff/tools/tiffdump.c,v retrieving revision 1.28
diff -r1.28 tiffdump.c
376a377,378
> int datasizeoverflow;
>
414a417
> datasizeoverflow = (typewidth > 0 && datasize / typewidth != count);
421c424
< if (datasize>4)
---
> if (datasizeoverflow || datasize>4)
435c438
< if (datasize>8)
---
> if (datasizeoverflow || datasize>8)
445c448
< if (datasize>0x10000)
---
> if (datasizeoverflow || datasize>0x10000)
--
Spatialys - Geospatial professional services
http://www.spatialys.com