libtiff: Correct and harmonize swapping of internal 'tif_header' parameters.

From 3c8480e3b7247385250d2dffaacb0b9651941dfb Mon Sep 17 00:00:00 2001
From: Su Laus <[EMAIL REDACTED]>
Date: Sun, 24 Mar 2024 14:35:42 +0000
Subject: [PATCH] Correct and harmonize swapping of internal 'tif_header'
 parameters.

---
 libtiff/tif_dir.c        |   6 +-
 libtiff/tif_open.c       |  44 ++++----
 libtiff/tiffiop.h        |  30 +++---
 test/test_open_options.c | 220 ++++++++++++++++++++++++++++++++++++++-
 4 files changed, 257 insertions(+), 43 deletions(-)

diff --git a/libtiff/tif_dir.c b/libtiff/tif_dir.c
index cff6e0e9..bd8e661a 100644
--- a/libtiff/tif_dir.c
+++ b/libtiff/tif_dir.c
@@ -2266,9 +2266,11 @@ int TIFFUnlinkDirectory(TIFF *tif, tdir_t dirn)
     }
     else
     {
+        /* Need local swap because nextdir has to be used unswapped below. */
+        uint64_t nextdir64 = nextdir;
         if (tif->tif_flags & TIFF_SWAB)
-            TIFFSwabLong8(&nextdir);
-        if (!WriteOK(tif, &nextdir, sizeof(uint64_t)))
+            TIFFSwabLong8(&nextdir64);
+        if (!WriteOK(tif, &nextdir64, sizeof(uint64_t)))
         {
             TIFFErrorExtR(tif, module, "Error writing directory link");
             return (0);
diff --git a/libtiff/tif_open.c b/libtiff/tif_open.c
index 74b5532e..b18e8948 100644
--- a/libtiff/tif_open.c
+++ b/libtiff/tif_open.c
@@ -547,9 +547,9 @@ TIFF *TIFFClientOpenExt(const char *name, const char *mode,
             TIFFErrorExtR(tif, name, "Cannot read TIFF header");
             goto bad;
         }
-/*
- * Setup header and write.
- */
+        /*
+         * Setup header and write.
+         */
 #ifdef WORDS_BIGENDIAN
         tif->tif_header.common.tiff_magic =
             (tif->tif_flags & TIFF_SWAB) ? TIFF_LITTLEENDIAN : TIFF_BIGENDIAN;
@@ -557,13 +557,17 @@ TIFF *TIFFClientOpenExt(const char *name, const char *mode,
         tif->tif_header.common.tiff_magic =
             (tif->tif_flags & TIFF_SWAB) ? TIFF_BIGENDIAN : TIFF_LITTLEENDIAN;
 #endif
+        TIFFHeaderUnion tif_header_swapped;
         if (!(tif->tif_flags & TIFF_BIGTIFF))
         {
             tif->tif_header.common.tiff_version = TIFF_VERSION_CLASSIC;
             tif->tif_header.classic.tiff_diroff = 0;
-            if (tif->tif_flags & TIFF_SWAB)
-                TIFFSwabShort(&tif->tif_header.common.tiff_version);
             tif->tif_header_size = sizeof(TIFFHeaderClassic);
+            /* Swapped copy for writing */
+            _TIFFmemcpy(&tif_header_swapped, &tif->tif_header,
+                        sizeof(TIFFHeaderUnion));
+            if (tif->tif_flags & TIFF_SWAB)
+                TIFFSwabShort(&tif_header_swapped.common.tiff_version);
         }
         else
         {
@@ -571,12 +575,15 @@ TIFF *TIFFClientOpenExt(const char *name, const char *mode,
             tif->tif_header.big.tiff_offsetsize = 8;
             tif->tif_header.big.tiff_unused = 0;
             tif->tif_header.big.tiff_diroff = 0;
+            tif->tif_header_size = sizeof(TIFFHeaderBig);
+            /* Swapped copy for writing */
+            _TIFFmemcpy(&tif_header_swapped, &tif->tif_header,
+                        sizeof(TIFFHeaderUnion));
             if (tif->tif_flags & TIFF_SWAB)
             {
-                TIFFSwabShort(&tif->tif_header.common.tiff_version);
-                TIFFSwabShort(&tif->tif_header.big.tiff_offsetsize);
+                TIFFSwabShort(&tif_header_swapped.common.tiff_version);
+                TIFFSwabShort(&tif_header_swapped.big.tiff_offsetsize);
             }
-            tif->tif_header_size = sizeof(TIFFHeaderBig);
         }
         /*
          * The doc for "fopen" for some STD_C_LIBs says that if you
@@ -586,26 +593,12 @@ TIFF *TIFFClientOpenExt(const char *name, const char *mode,
          * on Solaris.
          */
         TIFFSeekFile(tif, 0, SEEK_SET);
-        if (!WriteOK(tif, &tif->tif_header, (tmsize_t)(tif->tif_header_size)))
+        if (!WriteOK(tif, &tif_header_swapped,
+                     (tmsize_t)(tif->tif_header_size)))
         {
             TIFFErrorExtR(tif, name, "Error writing TIFF header");
             goto bad;
         }
-        /*
-         * Setup the byte order handling.
-         */
-        if (tif->tif_header.common.tiff_magic == TIFF_BIGENDIAN)
-        {
-#ifndef WORDS_BIGENDIAN
-            tif->tif_flags |= TIFF_SWAB;
-#endif
-        }
-        else
-        {
-#ifdef WORDS_BIGENDIAN
-            tif->tif_flags |= TIFF_SWAB;
-#endif
-        }
         /*
          * Setup default directory.
          */
@@ -616,8 +609,9 @@ TIFF *TIFFClientOpenExt(const char *name, const char *mode,
         tif->tif_setdirectory_force_absolute = FALSE;
         return (tif);
     }
+
     /*
-     * Setup the byte order handling.
+     * Setup the byte order handling according to the opened file for reading.
      */
     if (tif->tif_header.common.tiff_magic != TIFF_BIGENDIAN &&
         tif->tif_header.common.tiff_magic != TIFF_LITTLEENDIAN
diff --git a/libtiff/tiffiop.h b/libtiff/tiffiop.h
index 08ba0dfe..519e8f59 100644
--- a/libtiff/tiffiop.h
+++ b/libtiff/tiffiop.h
@@ -102,6 +102,13 @@ struct TIFFOffsetAndDirNumber
 };
 typedef struct TIFFOffsetAndDirNumber TIFFOffsetAndDirNumber;
 
+typedef union
+{
+    TIFFHeaderCommon common;
+    TIFFHeaderClassic classic;
+    TIFFHeaderBig big;
+} TIFFHeaderUnion;
+
 struct tiff
 {
     char *tif_name; /* name of open file */
@@ -153,20 +160,15 @@ struct tiff
     TIFFDirectory tif_dir;               /* internal rep of current directory */
     TIFFDirectory
         tif_customdir; /* custom IFDs are separated from the main ones */
-    union
-    {
-        TIFFHeaderCommon common;
-        TIFFHeaderClassic classic;
-        TIFFHeaderBig big;
-    } tif_header;
-    uint16_t tif_header_size;  /* file's header block and its length */
-    uint32_t tif_row;          /* current scanline */
-    tdir_t tif_curdir;         /* current directory (index) */
-    uint32_t tif_curstrip;     /* current strip for read/write */
-    uint64_t tif_curoff;       /* current offset for read/write */
-    uint64_t tif_lastvalidoff; /* last valid offset allowed for rewrite in
-                                  place. Used only by TIFFAppendToStrip() */
-    uint64_t tif_dataoff;      /* current offset for writing dir */
+    TIFFHeaderUnion tif_header; /* file's header block Classic/BigTIFF union */
+    uint16_t tif_header_size;   /* file's header block and its length */
+    uint32_t tif_row;           /* current scanline */
+    tdir_t tif_curdir;          /* current directory (index) */
+    uint32_t tif_curstrip;      /* current strip for read/write */
+    uint64_t tif_curoff;        /* current offset for read/write */
+    uint64_t tif_lastvalidoff;  /* last valid offset allowed for rewrite in
+                                   place. Used only by TIFFAppendToStrip() */
+    uint64_t tif_dataoff;       /* current offset for writing dir */
     /* SubIFD support */
     uint16_t tif_nsubifd;   /* remaining subifds to write */
     uint64_t tif_subifdoff; /* offset for patching SubIFD link */
diff --git a/test/test_open_options.c b/test/test_open_options.c
index 4a3e0a22..f0a8332d 100644
--- a/test/test_open_options.c
+++ b/test/test_open_options.c
@@ -271,11 +271,221 @@ static int test_TIFFOpenOptionsSetMaxCumulatedMemAlloc(
     return ret;
 }
 
+int test_header_byte_order(TIFF *tif, char *openModeString)
+{
+
+    if (tif->tif_header.common.tiff_magic != TIFF_BIGENDIAN &&
+        tif->tif_header.common.tiff_magic != TIFF_LITTLEENDIAN)
+    {
+        fprintf(stderr,
+                "Bad magic number %" PRIu16 " (0x%4.4" PRIx16 ") for '%s'\n",
+                tif->tif_header.common.tiff_magic,
+                tif->tif_header.common.tiff_magic, openModeString);
+        return 1;
+    }
+    if ((tif->tif_header.common.tiff_version != TIFF_VERSION_CLASSIC) &&
+        (tif->tif_header.common.tiff_version != TIFF_VERSION_BIG))
+    {
+        fprintf(stderr,
+                "Bad version number %" PRIu16 " (0x%4.4" PRIx16
+                ") for '%s'. Should be 0x%4.4" PRIx16 ".\n",
+                tif->tif_header.common.tiff_version,
+                tif->tif_header.common.tiff_version, openModeString,
+                TIFF_VERSION_CLASSIC);
+        return 1;
+    }
+    if (TIFFIsBigTIFF(tif))
+    {
+        if (tif->tif_header.big.tiff_offsetsize != 8)
+        {
+            fprintf(stderr,
+                    "Bad BigTIFF offset-size %" PRIu16 " (0x%4.4" PRIx16
+                    ") for '%s'. Should be 8.\n",
+                    tif->tif_header.big.tiff_offsetsize,
+                    tif->tif_header.big.tiff_offsetsize, openModeString);
+            return 1;
+        }
+    }
+
+    return 0;
+}
+
+int open_file_and_write(const char *filename, char *openModeString,
+                        int blnWriteIFD, int blnClose, TIFF **tifOut)
+{
+    TIFF *tif = TIFFOpen(filename, openModeString);
+    if (tifOut != NULL)
+        *tifOut = tif;
+    if (!tif)
+    {
+        fprintf(stderr, "Can't create or open %s\n", filename);
+        return 0x0100;
+    }
+    int ret = test_header_byte_order(tif, openModeString);
+    if (blnWriteIFD)
+    {
+        unsigned char buf[3] = {253, 127, 255};
+        TIFFSetField(tif, TIFFTAG_IMAGEWIDTH, 1);
+        TIFFSetField(tif, TIFFTAG_IMAGELENGTH, 1);
+        TIFFSetField(tif, TIFFTAG_SAMPLESPERPIXEL, 1);
+        TIFFSetField(tif, TIFFTAG_BITSPERSAMPLE, 8);
+        TIFFWriteScanline(tif, buf, 0, 0);
+        if (!TIFFWriteDirectory(tif))
+        {
+            fprintf(stderr, "TIFFWriteDirectory() failed for %s.\n", filename);
+            TIFFClose(tif);
+            if (tifOut != NULL)
+                *tifOut = NULL;
+            return 0x1000;
+        }
+        if (test_header_byte_order(tif, openModeString))
+            ret += 0x0010;
+    }
+    if (blnClose)
+    {
+        TIFFClose(tif);
+        if (tifOut != NULL)
+            *tifOut = NULL;
+    }
+    return ret;
+}
+
+/* Checks that parameters of internal 'tif_header' structure are all swapped to
+ * host byte-order.
+ *
+ * Till LibTIFF 4.6.0 the parameters 'tiff_magic', 'tiff_version' and
+ * 'tiff_offsetsize' are swapped or not, depending how the file was opened: They
+ * were not swapped when opening with "w" or opening an not existent file with
+ * "a". They were swapped when opening an existent file with "r" or "a". This
+ * behaviour shall be harmonized so that the internal parameters are allways in
+ * host byte-order.
+ */
+int test_TIFFheader_swapping(void)
+{
+
+    /* Provoke swapping and non-swapping on any host. */
+    char *openModeStrings[2][8] = {{"wb", "ab", "w8b", "a8b", "r"},
+                                   {"wl", "al", "w8l", "a8l", "r"}};
+    const char *filename = "test_TIFFheader_swapping.tif";
+    TIFF *tif;
+    int ret = 0;
+
+    unlink(filename);
+
+    for (unsigned int i = 0; i < 2; i++)
+    {
+        for (unsigned int k = 0; k < 4; k += 2)
+        {
+            /* Classic TIFF an BigTIFF */
+            /* Before fix, 'wb' and 'wb8' failed on little-endian hosts,
+             * and 'wl' and 'wl8' failed on big-endian hosts. */
+            unsigned int kk = k; /* Existing read-only */
+            if (open_file_and_write(filename, openModeStrings[i][kk], FALSE,
+                                    TRUE, &tif))
+            {
+                fprintf(stderr,
+                        "TIFF header byte-order in opening mode '%s' failed.\n",
+                        openModeStrings[i][kk]);
+                ret++;
+            }
+            kk++; /* Existing in append */
+            if (open_file_and_write(filename, openModeStrings[i][kk], TRUE,
+                                    TRUE, &tif))
+            {
+                fprintf(stderr,
+                        "TIFF header byte-order in opening mode '%s' failed.\n",
+                        openModeStrings[i][kk]);
+                ret++;
+            }
+
+            kk = 4; /* Existing read-only */
+            if (open_file_and_write(filename, openModeStrings[i][kk], FALSE,
+                                    TRUE, &tif))
+            {
+                fprintf(stderr,
+                        "TIFF header byte-order in opening mode '%s' failed.\n",
+                        openModeStrings[i][kk]);
+                ret++;
+            }
+            unlink(filename);
+        }
+
+        for (unsigned int k = 1; k < 4; k += 2)
+        {
+            /* Classic and BigTIFF:
+             * Open and write two IFD with non-existing file in append
+             * mode. Test also TIFFUnlinkDirectory(1) for known bug. */
+            /* Before fix, 'wb' and 'wb8' failed on little-endian hosts,
+             * and 'wl' and 'wl8' failed on big-endian hosts. */
+            if (open_file_and_write(filename, openModeStrings[i][k], TRUE, TRUE,
+                                    &tif))
+            {
+                fprintf(stderr,
+                        "TIFF header byte-order in opening mode '%s' failed.\n",
+                        openModeStrings[i][k]);
+                ret++;
+            }
+            /* Write second IFD for unlinking of first IFD and keep file open.
+             */
+            if (open_file_and_write(filename, openModeStrings[i][k], TRUE,
+                                    FALSE, &tif))
+            {
+                fprintf(stderr,
+                        "TIFF header byte-order in opening mode '%s' failed.\n",
+                        openModeStrings[i][k]);
+                ret++;
+            }
+            if (!TIFFUnlinkDirectory(tif, 1))
+            {
+                fprintf(stderr,
+                        "TIFFUnlinkDirectory(1) in opening mode '%s' failed.\n",
+                        openModeStrings[i][k]);
+                ret++;
+            }
+            if (test_header_byte_order(tif, openModeStrings[i][k]))
+            {
+                fprintf(
+                    stderr,
+                    "TIFF header byte-order after TIFFUnlinkDirectory(1) in "
+                    "opening mode '%s' failed.\n",
+                    openModeStrings[i][k]);
+                ret++;
+            }
+            /* Before fix, TIFFSetDirectory() failed when BigTIFF and swapping
+             * is needed, because TIFFUnlinkDirectory() stores tiff_diroff in
+             * wrong byte-order then. */
+            if (!TIFFSetDirectory(tif, 0))
+            {
+                fprintf(stderr,
+                        "TIFFSetDirectory(0) after TIFFUnlinkDirectory(1) in "
+                        "opening mode '%s' failed.\n",
+                        openModeStrings[i][k]);
+                ret++;
+            }
+            TIFFClose(tif);
+            unlink(filename);
+        }
+    }
+
+    unlink(filename);
+    return ret;
+}
+
 int main()
 {
     int ret = 0;
+    fprintf(stderr, "=== test_open_options .... ===\n");
+    fprintf(stderr, "---- test_TIFFheader_swapping ---- \n");
+    if (ret += test_TIFFheader_swapping())
+    {
+        fprintf(stderr,
+                "--- Failed during test_TIFFheader_swapping test with %d "
+                "fails. ---\n",
+                ret);
+    }
+
     ret += test_error_handler();
-    fprintf(stderr, "---- test_TIFFOpenOptionsSetMaxSingleMemAlloc "
+    fprintf(stderr, "\n---- test_TIFFOpenOptionsSetMaxSingleMemAlloc "
                     "with non-BigTIFF ---- \n");
     ret += test_TIFFOpenOptionsSetMaxSingleMemAlloc(1, TRUE, -1, FALSE);
     ret += test_TIFFOpenOptionsSetMaxSingleMemAlloc(
@@ -292,13 +502,19 @@ int main()
     ret += test_TIFFOpenOptionsSetMaxSingleMemAlloc(
         VALUE_SAMPLESPERPIXEL * sizeof(short), FALSE, FALSE, TRUE);
 
-    fprintf(stderr, "---- test_TIFFOpenOptionsSetMaxCumulatedMemAlloc ---- \n");
+    fprintf(stderr,
+            "\n---- test_TIFFOpenOptionsSetMaxCumulatedMemAlloc ---- \n");
     ret += test_TIFFOpenOptionsSetMaxCumulatedMemAlloc(1, TRUE, -1);
     ret += test_TIFFOpenOptionsSetMaxCumulatedMemAlloc(
         sizeof(TIFF) + strlen("test_error_handler.tif") + 1, FALSE, TRUE);
     ret += test_TIFFOpenOptionsSetMaxCumulatedMemAlloc(
         sizeof(TIFF) + strlen("test_error_handler.tif") + 1 + 30000, FALSE,
         FALSE);
+    if (ret > 0)
+        fprintf(stderr, "\n=== test_open_options failed with %d fails ===\n",
+                ret);
+    else
+        fprintf(stderr, "\n=== test_open_options finished OK ===\n");
 
     return ret;
 }