libtiff: rework update-in-place logic substantially to make safer

From 6bacd7ec6482ba5b16e3a52937151d852b8e3265 Mon Sep 17 00:00:00 2001
From: Frank Warmerdam <[EMAIL REDACTED]>
Date: Thu, 22 Nov 2007 22:24:59 +0000
Subject: [PATCH] rework update-in-place logic substantially to make safer

---
 ChangeLog           |  10 ++++
 libtiff/tif_write.c | 118 +++++++++++++++++++-------------------------
 2 files changed, 61 insertions(+), 67 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 29000bd3..5f139298 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,15 @@
 2007-11-22  Frank Warmerdam  <warmerdam@pobox.com>
 
+	* tif_write.c: Rip out the fancy logic in TIFFAppendToStrip() for 
+	establishing if an existing tile can be rewritten to the same location 
+	by comparing the current size to all the other blocks in the same 
+	directory.  This is dangerous in many situations and can easily 
+	corrupt a file.  (observed in esoteric GDAL situation that's hard to
+	document).  This change involves leaving the stripbytecount[] values 
+	unaltered till TIFFAppendToStrip().  Now we only write a block back
+	to the same location it used to be at if the new data is the same
+	size or smaller - otherwise we move it to the end of file.
+
 	* tif_dirwrite.c: Try to avoid writing out a full readbuffer of tile
 	data when writing the directory just because we have BEENWRITING at
 	some point in the past.  This was causing odd junk to be written out
diff --git a/libtiff/tif_write.c b/libtiff/tif_write.c
index 85b17314..26a2199d 100644
--- a/libtiff/tif_write.c
+++ b/libtiff/tif_write.c
@@ -1,4 +1,4 @@
-/* $Id: tif_write.c,v 1.22 2006-11-20 02:11:41 fwarmerdam Exp $ */
+/* $Id: tif_write.c,v 1.22.2.1 2007-11-22 22:25:00 fwarmerdam Exp $ */
 
 /*
  * Copyright (c) 1988-1997 Sam Leffler
@@ -227,10 +227,8 @@ TIFFWriteEncodedStrip(TIFF* tif, tstrip_t strip, tdata_t data, tsize_t cc)
 
         if( td->td_stripbytecount[strip] > 0 )
         {
-            /* if we are writing over existing tiles, zero length. */
-            td->td_stripbytecount[strip] = 0;
-
-            /* this forces TIFFAppendToStrip() to do a seek */
+	    /* Force TIFFAppendToStrip() to consider placing data at end
+               of file. */
             tif->tif_curoff = 0;
         }
         
@@ -363,10 +361,8 @@ TIFFWriteEncodedTile(TIFF* tif, ttile_t tile, tdata_t data, tsize_t cc)
 
         if( td->td_stripbytecount[tile] > 0 )
         {
-            /* if we are writing over existing tiles, zero length. */
-            td->td_stripbytecount[tile] = 0;
-
-            /* this forces TIFFAppendToStrip() to do a seek */
+	    /* Force TIFFAppendToStrip() to consider placing data at end
+               of file. */
             tif->tif_curoff = 0;
         }
         
@@ -625,67 +621,55 @@ TIFFGrowStrips(TIFF* tif, int delta, const char* module)
 static int
 TIFFAppendToStrip(TIFF* tif, tstrip_t strip, tidata_t data, tsize_t cc)
 {
-    TIFFDirectory *td = &tif->tif_dir;
-    static const char module[] = "TIFFAppendToStrip";
-
-    if (td->td_stripoffset[strip] == 0 || tif->tif_curoff == 0) {
-        /*
-         * No current offset, set the current strip.
-         */
-        assert(td->td_nstrips > 0);
-        if (td->td_stripoffset[strip] != 0) {
-            /*
-             * Prevent overlapping of the data chunks. We need
-             * this to enable in place updating of the compressed
-             * images. Larger blocks will be moved at the end of
-             * the file without any optimization of the spare
-             * space, so such scheme is not too much effective.
-             */
-            if (td->td_stripbytecountsorted) {
-                if (strip == td->td_nstrips - 1
-                    || td->td_stripoffset[strip + 1] <
-                    td->td_stripoffset[strip] + cc) {
-                    td->td_stripoffset[strip] =
-                        TIFFSeekFile(tif, (toff_t)0,
-                                     SEEK_END);
-                    td->td_stripbytecountsorted = 0;
-                }
-            } else {
-                tstrip_t i;
-                for (i = 0; i < td->td_nstrips; i++) {
-                    if (td->td_stripoffset[i] > 
-                        td->td_stripoffset[strip]
-                        && td->td_stripoffset[i] <
-                        td->td_stripoffset[strip] + cc) {
-                        td->td_stripoffset[strip] =
-                            TIFFSeekFile(tif,
-                                         (toff_t)0,
-                                         SEEK_END);
-                    }
+	static const char module[] = "TIFFAppendToStrip";
+	TIFFDirectory *td = &tif->tif_dir;
+
+	if (td->td_stripoffset[strip] == 0 || tif->tif_curoff == 0) {
+            assert(td->td_nstrips > 0);
+
+            if( td->td_stripbytecount[strip] != 0 
+                && td->td_stripoffset[strip] != 0 
+                && td->td_stripbytecount[strip] >= cc )
+            {
+                /* 
+                 * There is already tile data on disk, and the new tile
+                 * data we have to will fit in the same space.  The only 
+                 * aspect of this that is risky is that there could be
+                 * more data to append to this strip before we are done
+                 * depending on how we are getting called.
+                 */
+                if (!SeekOK(tif, td->td_stripoffset[strip])) {
+                    TIFFErrorExt(tif->tif_clientdata, module,
+                                 "Seek error at scanline %lu",
+                                 (unsigned long)tif->tif_row);
+                    return (0);
                 }
             }
-
-            if (!SeekOK(tif, td->td_stripoffset[strip])) {
-                TIFFErrorExt(tif->tif_clientdata, module,
-                             "%s: Seek error at scanline %lu",
-                             tif->tif_name,
-                             (unsigned long)tif->tif_row);
-                return (0);
+            else
+            {
+                /* 
+                 * Seek to end of file, and set that as our location to 
+                 * write this strip.
+                 */
+                td->td_stripoffset[strip] = TIFFSeekFile(tif, 0, SEEK_END);
             }
-        } else
-            td->td_stripoffset[strip] =
-                TIFFSeekFile(tif, (toff_t) 0, SEEK_END);
-        tif->tif_curoff = td->td_stripoffset[strip];
-    }
-
-    if (!WriteOK(tif, data, cc)) {
-        TIFFErrorExt(tif->tif_clientdata, module, "%s: Write error at scanline %lu",
-                     tif->tif_name, (unsigned long) tif->tif_row);
-        return (0);
-    }
-    tif->tif_curoff += cc;
-    td->td_stripbytecount[strip] += cc;
-    return (1);
+
+            tif->tif_curoff = td->td_stripoffset[strip];
+
+            /*
+             * We are starting a fresh strip/tile, so set the size to zero.
+             */
+            td->td_stripbytecount[strip] = 0;
+	}
+
+	if (!WriteOK(tif, data, cc)) {
+		TIFFErrorExt(tif->tif_clientdata, module, "Write error at scanline %lu",
+		    (unsigned long) tif->tif_row);
+		    return (0);
+	}
+	tif->tif_curoff =  tif->tif_curoff+cc;
+	td->td_stripbytecount[strip] += cc;
+	return (1);
 }
 
 /*