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;
}