2012.01.07 02:58 "[Tiff] Proposed patch for tif_fax3 for not ignoring return values from failed FlushData", by Ryan Wong

This proposed patch addresses an issue with some tif_fax3 macros which call TIFFFlushData1, in which:   - the return code from TIFFFlushData1 was ignored  - execution was allowed to continue  - the buffer write pointer, "tif->tif_rawcp" was not being reset to the beginning (that is, pointing past the end of the buffer)  - the continued execution of the code will lead to the bytes beyond the buffer being overwritten. Normally, it is rare for TIFFFlushData1 to fail. It is more likely to happen if a customized TIFF data I/O interface was used. This has been observed once in an internal-use non-published software. For the sake of responsible disclosure, the condition which triggers this issue is being withheld. Detailed modifications:   - The functions which contain the macros, Fax3FlushBits and _FlushBits, are modified so that they return an integer value.    - 1 is returned for success. 0 is returned for failure. This follows from the convention of TIFFFlushData1.    - Fax3PutBits, putspan, Fax3PutEOL,     - a function was added, Fax3FlushBitsReturn, for the purpose of ignoring the return code from the macro when it is called from Fax3Close. The patch also contains a modification of Fax3Unexpected() function and the unexpected() macro, for printing additional information (the "table"). This is unrelated to the TIFFFlushData1 macro issue. Your opinions are greatly appreciated. In particular, we would like to know if there is any risk introduced by this patch, so that we can address them. Regards,rwong_002@hotmail.com                                       

--- libtiff-3_9_5_r227596\source\libtiff\tif_fax3.c     2012-01-06 17:40:59.745694300 -0800

+++ libtiff-3_9_5\source\libtiff\tif_fax3.c     2012-01-06 17:49:50.098760000 -0800

@@ -178,20 +178,20 @@

*/

static void

-Fax3Unexpected(const char* module, TIFF* tif, uint32 line, uint32 a0)
+Fax3Unexpected(const char* module, const char* table, TIFF* tif, uint32 line, uint32 a0)
 {

-       TIFFErrorExt(tif->tif_clientdata, module, "%s: Bad code word at line %u of %s %u (x %u)",
+       TIFFErrorExt(tif->tif_clientdata, module, "%s: Bad code word at line %u of %s %u (x %u, table %s)",

                     tif->tif_name, line, isTiled(tif)? "tile": "strip",
                     (isTiled(tif)? tif->tif_curtile: tif->tif_curstrip),
- a0);
+ a0, table);
 }

-#define        unexpected(table, a0)   Fax3Unexpected(module, tif, sp->line, a0)
+#define        unexpected(table, a0)   Fax3Unexpected(module, table, tif, sp->line, a0)

 static void
 Fax3Extension(const char* module, TIFF* tif, uint32 line, uint32 a0)
 {
        TIFFErrorExt(tif->tif_clientdata, module,
                     "%s: Uncompressed data (not supported) at line %u of %s %u (x %u)",
@@ -551,21 +551,27 @@

/*
 * CCITT Group 3 FAX Encoding.
 */

 #define        Fax3FlushBits(tif, sp) {                                \
-       if ((tif)->tif_rawcc >= (tif)->tif_rawdatasize)         \
-               (void) TIFFFlushData1(tif);                     \
+       if ((tif)->tif_rawcc >= (tif)->tif_rawdatasize) {    \
+               if (1 != TIFFFlushData1(tif) ||                  \
+                   (tif)->tif_rawcc >= (tif)->tif_rawdatasize)  \
+               return 0;    \
+       }    \
        *(tif)->tif_rawcp++ = (tidataval_t) (sp)->data;         \
        (tif)->tif_rawcc++;                                     \
        (sp)->data = 0, (sp)->bit = 8;                          \
 }
 #define        _FlushBits(tif) {                                       \
-       if ((tif)->tif_rawcc >= (tif)->tif_rawdatasize)         \
-               (void) TIFFFlushData1(tif);                     \
+       if ((tif)->tif_rawcc >= (tif)->tif_rawdatasize) {    \
+               if (1 != TIFFFlushData1(tif) ||                  \
+                   (tif)->tif_rawcc >= (tif)->tif_rawdatasize)  \
+               return 0;    \
+       }    \
        *(tif)->tif_rawcp++ = (tidataval_t) data;               \
        (tif)->tif_rawcc++;                                     \
        data = 0, bit = 8;                                      \

 }
 static const int _msbmask[9] =
     { 0x00, 0x01, 0x03, 0x07, 0x0f, 0x1f, 0x3f, 0x7f, 0xff };
@@ -583,23 +589,24 @@

/*

  */
-static void
+static int
 Fax3PutBits(TIFF* tif, unsigned int bits, unsigned int length)
 {
        Fax3CodecState* sp = EncoderState(tif);
        unsigned int bit = sp->bit;
        int data = sp->data;

      _PutBits(tif, bits, length);

        sp->data = data;
        sp->bit = bit;
+ return 1;
 }

 /*
  * Write a code to the output stream.
  */
 #define putcode(tif, te) Fax3PutBits(tif, (te)->code, (te)->length)
@@ -618,13 +625,13 @@
 /*