Double free in tiffcrop loadImage() with large buffsize

Hi,

we have found a double free and would like to report this issue. Would this be considered a valid bug?

Summary

A double free occurs when a TIFF file is input that causes buffsize to become large.

Reproduction

Tested Environment

  • OS: Ubuntu 24.04.3 LTS
  • arch: x86_64
  • CC: gcc 14.2.0
  • glibc: 2.39

Reproduction Steps

# prepare input file
git clone https://github.com/momo-trip/input_tiffcrop

git clone https://gitlab.com/libtiff/libtiff
cp input_tiffcrop/input0.tiff libtiff
cd libtiff
./autogen.sh
./configure  CFLAGS="-g -fsanitize=address -O0"
make
tools/tiffcrop -z 0,0,32,32 input0.tiff /tmp/foo

Output

TIFFReadDirectoryCheckOrder: Warning, Invalid TIFF directory; tags are not sorted in ascending order.
TIFFFetchNormalTag: Warning, Incorrect count for "FillOrder"; tag ignored.
TIFFFetchNormalTag: Defined set_get_field_type of custom tag 1024 (Tag 1024) is TIFF_SETGET_UNDEFINED and thus tag is not read from file.
TIFFReadDirectory: Warning, TIFF directory is missing required "StripByteCounts" field, calculating from imagelength.
PackBitsDecode: Warning, Discarding 106 bytes to avoid buffer overrun.
TIFFReadDirectoryCheckOrder: Warning, Invalid TIFF directory; tags are not sorted in ascending order.
TIFFFetchNormalTag: Warning, Incorrect count for "Orientation"; tag ignored.
TIFFReadDirectory: Warning, Sum of Photometric type-related color channels and ExtraSamples doesn't match SamplesPerPixel. Defining non-color channels as ExtraSamples..
TIFFReadDirectory: Warning, TIFF directory is missing required "StripByteCounts" field, calculating from imagelength.
MemoryLimitError: allocation of 371982339 bytes is forbidden. Limit is 268435456.
                  use -k option to change limit.
loadImage: Unable to allocate read buffer.
main: Unable to load source image.
=================================================================
==3687412==ERROR: AddressSanitizer: attempting double-free on 0x519000000080 in thread T0:
    #0 0x741c8f0fc4d8 in free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:52
    #1 0x741c8ef8d51a in _TIFFfree /tmp/libtiff/libtiff/tif_unix.c:349
    #2 0x58b8a36f4dff in main /tmp/libtiff/tools/tiffcrop.c:2934
    #3 0x741c8ea2a1c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #4 0x741c8ea2a28a in __libc_start_main_impl ../csu/libc-start.c:360
    #5 0x58b8a36eab64 in _start (/tmp/libtiff/tools/.libs/tiffcrop+0x9b64) (BuildId: fa8fa2b35ce15a0992c252ce9700c2f1ebcbc0f5)

0x519000000080 is located 0 bytes inside of 995-byte region [0x519000000080,0x519000000463)
freed by thread T0 here:
    #0 0x741c8f0fc4d8 in free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:52
    #1 0x741c8ef8d51a in _TIFFfree /tmp/libtiff/libtiff/tif_unix.c:349
    #2 0x58b8a370ca2e in loadImage /tmp/libtiff/tools/tiffcrop.c:7201
    #3 0x58b8a36f431f in main /tmp/libtiff/tools/tiffcrop.c:2795
    #4 0x741c8ea2a1c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #5 0x741c8ea2a28a in __libc_start_main_impl ../csu/libc-start.c:360
    #6 0x58b8a36eab64 in _start (/tmp/libtiff/tools/.libs/tiffcrop+0x9b64) (BuildId: fa8fa2b35ce15a0992c252ce9700c2f1ebcbc0f5)

previously allocated by thread T0 here:
    #0 0x741c8f0fd9c7 in malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x741c8ef8d4be in _TIFFmalloc /tmp/libtiff/libtiff/tif_unix.c:338
    #2 0x58b8a36ead03 in limitMalloc /tmp/libtiff/tools/tiffcrop.c:709
    #3 0x58b8a370ca77 in loadImage /tmp/libtiff/tools/tiffcrop.c:7209
    #4 0x58b8a36f431f in main /tmp/libtiff/tools/tiffcrop.c:2795
    #5 0x741c8ea2a1c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #6 0x741c8ea2a28a in __libc_start_main_impl ../csu/libc-start.c:360
    #7 0x58b8a36eab64 in _start (/tmp/libtiff/tools/.libs/tiffcrop+0x9b64) (BuildId: fa8fa2b35ce15a0992c252ce9700c2f1ebcbc0f5)

SUMMARY: AddressSanitizer: double-free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:52 in free
==3687412==ABORTING

Root Cause Analysis

  1. [tiffcrop.c:7193-7202](https://gitlab.com/libtiff/libtiff/-/blob/master/tools/tiffcrop.c?ref_type=heads#L7193-7202)
    read_buff = *read_ptr;
    if (read_buff)
    {
        _TIFFfree(read_buff);
    }

Here, *read_ptr is freed once.

  1. [tiffcrop.c:7208-7214](https://gitlab.com/libtiff/libtiff/-/blob/master/tools/tiffcrop.c?ref_type=heads#L7208-7214)
    read_buff =
        (unsigned char *)limitMalloc(buffsize + NUM_BUFF_OVERSIZE_BYTES);
    if (!read_buff)
    {
        TIFFError("loadImage", "Unable to allocate read buffer");
        return (-1);
    }

If the allocation here fails, the function returns with the old value still in *read_ptr.

  1. [tiffcrop.c:2933-2934](https://gitlab.com/libtiff/libtiff/-/blob/master/tools/tiffcrop.c?ref_type=heads#L2933-2934)
    if (read_buff && read_buff != crop_buff)
        _TIFFfree(read_buff);

Here, read_buff is freed again, causing a double free.

[tiffcrop.c:7203-7207](https://gitlab.com/libtiff/libtiff/-/blob/master/tools/tiffcrop.c?ref_type=heads#L7203-7207)

    if (buffsize > 0xFFFFFFFFU - 3)
    {
        TIFFError("loadImage", "Required read buffer size too large");
        return (-1);
    }

To trigger the double free, the TIFF's SamplesPerPixel value needs to be large enough to cause malloc to fail. The SamplesPerPixel value is used in the calculation of buffsize. The code returns here when buffsize exceeds 0xFFFFFFFFU - NUM_BUFF_OVERSIZE_BYTES, but the same double free occurs in this case as well.

Proposed fix

This issue can be avoided by setting read_ptr to NULL before returning when there's a problem with buffsize.

diff --git a/tools/tiffcrop.c b/tools/tiffcrop.c
index aac385e5..8148ffdd 100644
--- a/tools/tiffcrop.c
+++ b/tools/tiffcrop.c
@@ -7203,6 +7203,7 @@ static int loadImage(TIFF *in, struct image_data *image, struct dump_opts *dump,
     if (buffsize > 0xFFFFFFFFU - 3)
     {
         TIFFError("loadImage", "Required read buffer size too large");
+        *read_ptr = NULL;
         return (-1);
     }
     read_buff =
@@ -7210,6 +7211,7 @@ static int loadImage(TIFF *in, struct image_data *image, struct dump_opts *dump,
     if (!read_buff)
     {
         TIFFError("loadImage", "Unable to allocate read buffer");
+        *read_ptr = NULL;
         return (-1);
     }
     check_buffsize = buffsize + NUM_BUFF_OVERSIZE_BYTES;

Related issue

#738 (closed)