2017.08.02 15:00 "[Tiff] Error handling in Read/Write/Seek", by Nicolas RUFF

Hello,

Apparently TIFFReadFile / TIFFWriteFile / TIFFSeekFile return byte count on success, and -1 on error.

I have a pathological TIFF file (generated by a fuzzer) where a directory seek will actually end up calling TIFFSeekFile(-1, SEEK_SET). This will pass the SeekOK() test in any case, and result in an out-of-bound access to tiff_buffer[-1].

The quick-and-dirty fix I have is redefining the following macros:

#define ReadOK(tif, buf, size) \ (TIFFReadFile((tif),(buf),(size))==(size)) #define SeekOK(tif, off) \ (TIFFSeekFile((tif),(off),SEEK_SET)==(off)) #define WriteOK(tif, buf, size) \ (TIFFWriteFile((tif),(buf),(size))==(size))

into:

#define ReadOK(tif, buf, size) \ ((TIFFReadFile((tif),(buf),(size))==(size) && (size)!=-1)) #define SeekOK(tif, off) \ ((TIFFSeekFile((tif),(off),SEEK_SET)==(off) && (off)!=-1)) #define WriteOK(tif, buf, size) \ ((TIFFWriteFile((tif),(buf),(size))==(size) && (size)!=-1))

This approach has two shortcomings:

1. If "size" or "off" is an expression, it will be evaluated one more time.
2. I wonder if a directory entry could be located at exact offset

0xFFFFFFFF in the input file (in which case it would be inaccessible).

The real fix would be some kind of out-of-band error reporting (a la errno). Such a change would be transparent to all users of ReadOK / WriteOK / SeekOK macros, but I wonder if there are people in the wild checking return values on their own.

WDYT?