From 10083d2e74cb3427fd242d62cd19e5459a19e70e Mon Sep 17 00:00:00 2001 From: Bill Denney Date: Mon, 24 Aug 2026 14:42:30 +0000 Subject: [PATCH 1/2] Close the TIFF on writeTIFF() error paths to prevent a later crash Fixes https://github.com/s-u/tiff/issues/13 write_tiff() opens the TIFF at src/write.c:66 with a pointer to the stack-allocated tiff_job_t declared at src/write.c:31, but TIFFClose() is only reached on the success path. Every Rf_error() raised after TIFF_Open() therefore longjmps back to R leaving the TIFF open, the output FILE* unclosed, and common.c's static last_tiff pointing at a TIFF whose client data is a stack frame that no longer exists. That stale pointer is what crashes. TIFF_Open() assigns last_tiff only after TIFFClientOpen() returns, so when a later readTIFF() hits an unreadable header, libtiff raises the error from inside TIFFClientOpen() while last_tiff still refers to the dead write job. TIFFErrorHandler_() then calls TIFFClose(last_tiff) on it and dereferences the unwound frame. This also explains why only a failing read crashes: a successful read never invokes the error handler, so it merely overwrites last_tiff. Reproducible example (crashed 10/10 runs before this change, with the symptom alternating between "caught segfault", "free(): invalid size" and "glibc detected an invalid stdio handle"; clean 20/20 after): bad <- tempfile(fileext = ".tif") good <- tempfile(fileext = ".tif") file.create(good) try(tiff::writeTIFF(array(runif(80), dim = c(4L, 4L, 5L)), bad), silent = TRUE) tiff::readTIFF(good) Route the five Rf_error() calls sited after TIFF_Open() through a helper that closes the TIFF first. TIFFClose() runs TIFFCloseProc_(), which fcloses the file or frees the in-memory buffer and resets last_tiff, so it is the complete cleanup for both output targets. read.c already uses this same TIFFClose()-then-Rf_error() pattern at lines 380 and 385; this applies it to write.c. Error messages and all success paths are unchanged. Co-Authored-By: Claude Opus 5 --- src/write.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/src/write.c b/src/write.c index 60cc985..3488513 100644 --- a/src/write.c +++ b/src/write.c @@ -26,6 +26,24 @@ int analyze_native(const unsigned int *what, int length) { return alpha | (ac << 1); } +/* Close the TIFF before raising an R error. + + Rf_error() performs a longjmp, so an error raised after TIFF_Open() would + otherwise return to R with the TIFF still open. That matters because the + TIFF's client data is the stack-allocated tiff_job_t below, and because + common.c keeps the most recently opened TIFF in the static `last_tiff`. + The next libtiff error - e.g. a subsequent readTIFF() on a file with an + unreadable header - makes TIFFErrorHandler_() call TIFFClose(last_tiff) + on that stale TIFF, dereferencing a stack frame that no longer exists. + + TIFFClose() also runs TIFFCloseProc_(), which fcloses the output file (or + frees the in-memory buffer) and resets last_tiff, so this is the complete + cleanup for both output targets. */ +static void tiff_close_and_error(TIFF *tiff, const char *msg) { + TIFFClose(tiff); + Rf_error("%s", msg); +} + SEXP write_tiff(SEXP image, SEXP where, SEXP sBPS, SEXP sCompr, SEXP sReduce) { SEXP dims, img_list = 0; tiff_job_t rj; @@ -81,11 +99,11 @@ SEXP write_tiff(SEXP image, SEXP where, SEXP sBPS, SEXP sCompr, SEXP sReduce) { raw_array = 1; if (!native && !raw_array && TYPEOF(image) != REALSXP) - Rf_error("image must be a matrix or array of raw or real numbers"); + tiff_close_and_error(tiff, "image must be a matrix or array of raw or real numbers"); dims = Rf_getAttrib(image, R_DimSymbol); if (dims == R_NilValue || TYPEOF(dims) != INTSXP || LENGTH(dims) < 2 || LENGTH(dims) > 3) - Rf_error("image must be a matrix or an array of two or three dimensions"); + tiff_close_and_error(tiff, "image must be a matrix or an array of two or three dimensions"); if (raw_array && LENGTH(dims) == 3) { /* raw arrays have either bpp, width, height or width, height dimensions */ planes = INTEGER(dims)[0]; @@ -99,7 +117,7 @@ SEXP write_tiff(SEXP image, SEXP where, SEXP sBPS, SEXP sCompr, SEXP sReduce) { } if (planes < 1 || planes > 4) - Rf_error("image must have either 1 (grayscale), 2 (GA), 3 (RGB) or 4 (RGBA) planes"); + tiff_close_and_error(tiff, "image must have either 1 (grayscale), 2 (GA), 3 (RGB) or 4 (RGBA) planes"); if (native) { /* nativeRaster should have a "channels" attribute if it has anything else than 4 channels */ SEXP cha = getAttrib(image, install("channels")); @@ -112,7 +130,7 @@ SEXP write_tiff(SEXP image, SEXP where, SEXP sBPS, SEXP sCompr, SEXP sReduce) { } if (raw_array) { if (planes != 4) - Rf_error("Only RGBA format is supported as raw data"); + tiff_close_and_error(tiff, "Only RGBA format is supported as raw data"); native = 1; /* from now on we treat raw arrays like native */ } @@ -200,7 +218,7 @@ SEXP write_tiff(SEXP image, SEXP where, SEXP sBPS, SEXP sCompr, SEXP sReduce) { data16 = (unsigned short*) buf; data32 = (unsigned int*) buf; if (!buf) - Rf_error("cannot allocate output image buffer"); + tiff_close_and_error(tiff, "cannot allocate output image buffer"); if (bps == 8) for (y = 0; y < height; y++) for (x = 0; x < width; x++) From 8f9fe1753bd7e7baf12f3f262e8c347e7a878d59 Mon Sep 17 00:00:00 2001 From: Bill Denney Date: Mon, 24 Aug 2026 14:58:38 +0000 Subject: [PATCH 2/2] Close the TIFF on readTIFF() tiled-image error paths Follow-on to the writeTIFF() fix for https://github.com/s-u/tiff/issues/13, which reported the crash only via write_tiff(). read_tiff() has two instances of the same defect. The tiled-image branch raises errors at src/read.c:543 and :546 without closing the TIFF first, unlike the equivalent checks at :380 and :385. As in write_tiff(), the TIFF's client data is the stack-allocated tiff_job_t at src/read.c:173, and common.c's static last_tiff keeps pointing at it after Rf_error() longjmps out. The next libtiff error makes TIFFErrorHandler_() call TIFFClose(last_tiff) on a frame that no longer exists. Confirmed against a build that already carries the write_tiff() fix, so this is an independent reproducer: ## a 16x16 tiled RGB TIFF with PlanarConfiguration = SEPARATE try(tiff::readTIFF("tiled_planar.tif"), silent = TRUE) #> "Planar format tiled images are not supported" f <- tempfile(fileext = ".tif"); file.create(f) tiff::readTIFF(f) #> *** caught segfault *** Crashed 15/15 runs before this change, clean 15/15 after. As with the write path, following the failed read with a *successful* read is harmless: a successful open never invokes the error handler, so it only overwrites last_tiff rather than dereferencing it. Also close the file handle when TIFF_Open() itself returns NULL at src/read.c:202. That path is defensive rather than observed: libtiff reports open failures through TIFFErrorHandler_(), which calls Rf_error() and longjmps, so TIFF_Open() generally does not return at all on failure. Error messages and all success paths are unchanged. Co-Authored-By: Claude Opus 5 --- src/read.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/read.c b/src/read.c index b2ef7d9..93bba62 100644 --- a/src/read.c +++ b/src/read.c @@ -198,8 +198,10 @@ SEXP read_tiff(SEXP sFn, SEXP sNative, SEXP sAll, SEXP sConvert, SEXP sInfo, SEX } tiff = TIFF_Open("rmc", &rj); /* no mmap, no chopping */ - if (!tiff) + if (!tiff) { + if (f) fclose(f); Rf_error("Unable to open TIFF"); + } int cur_dir = 0; /* 1-based image number */ int nprot = 0; @@ -539,11 +541,15 @@ SEXP read_tiff(SEXP sFn, SEXP sNative, SEXP sAll, SEXP sConvert, SEXP sInfo, SEX } } } else { /* tiled image */ - if (indexed || colormap[0] || bps == 12) + if (indexed || colormap[0] || bps == 12) { + TIFFClose(tiff); Rf_error("Indexed and 12-bit tiled images are not supported."); + } - if (spp > 1 && config != PLANARCONFIG_CONTIG) + if (spp > 1 && config != PLANARCONFIG_CONTIG) { + TIFFClose(tiff); Rf_error("Planar format tiled images are not supported"); + } #ifdef TIFF_DEBUG Rprintf(" - %d x %d tiles\n", TIFFNumberOfTiles(tiff), TIFFTileSize(tiff));