TIFF image loading EXIF corruption

[Knio]

What did you do?

Load a TIFF with exif rotation info and save as JPG

What did you expect to happen?

Save image

What actually happened?

save() crashes

What are your OS, Python and Pillow versions?

• OS: Debian GNU/Linux 12 Linux dev 6.8.12-2-pve #1 SMP PREEMPT_DYNAMIC PMX 6.8.12-2 (2024-09-05T10:03Z) x86_64 GNU/Linux
• Python: Python 3.11.2 (main, Aug 26 2024, 07:20:54) [GCC 12.2.0]
• Pillow: 11.1.0.dev0

--------------------------------------------------------------------
Pillow 11.1.0.dev0
Python 3.11.2 (main, Aug 26 2024, 07:20:54) [GCC 12.2.0]
--------------------------------------------------------------------
Python executable is /usr/bin/python3
System Python files loaded from /usr
--------------------------------------------------------------------
Python Pillow modules loaded from /home/tom/.local/lib/python3.11/site-packages/PIL
Binary Pillow modules loaded from /home/tom/.local/lib/python3.11/site-packages/PIL
--------------------------------------------------------------------
--- PIL CORE support ok, compiled for 11.1.0.dev0
--- TKINTER support ok, loaded 8.6
--- FREETYPE2 support ok, loaded 2.12.1
--- LITTLECMS2 support ok, loaded 2.14
--- WEBP support ok, loaded 1.2.4
--- JPEG support ok, compiled for libjpeg-turbo 2.1.5
--- OPENJPEG (JPEG2000) support ok, loaded 2.5.2
--- ZLIB (PNG/ZIP) support ok, loaded 1.2.13
--- LIBTIFF support ok, loaded 4.5.0
*** RAQM (Bidirectional Text) support not installed
*** LIBIMAGEQUANT (Quantization method) support not installed
*** XCB (X protocol) support not installed
--------------------------------------------------------------------

img_path = pathlib.Path('IMG_0282.CR2')
img = PIL.Image.open(img_path)
img.save(img_path.with_suffix('.jpg'), format='JPEG')

Traceback (most recent call last):
  File "/mnt/zp/tom/docs/Programming/bugreports/pillow/pillow_bug2.py", line 27, in save
    img.save(img_path.with_suffix('.jpg'), format='JPEG')
  File "/home/tom/.local/lib/python3.11/site-packages/PIL/Image.py", line 2605, in save
    save_handler(self, fp, filename)
  File "/home/tom/.local/lib/python3.11/site-packages/PIL/JpegImagePlugin.py", line 843, in _save
    ImageFile._save(
  File "/home/tom/.local/lib/python3.11/site-packages/PIL/ImageFile.py", line 556, in _save
    _encode_tile(im, fp, tile, bufsize, fh)
  File "/home/tom/.local/lib/python3.11/site-packages/PIL/ImageFile.py", line 576, in _encode_tile
    encoder.setimage(im.im, extents)
SystemError: tile cannot extend outside image

Debugging Notes

I have debugged this, and determined it's caused by:

• save() calls load() calls into _load_libtiff()
• self.fp: _io.BufferedReader is corrupted after giving it to libtiff decode.decode()
• load_end() calls exif_transpose() calls getexif() calls ImageFileDirectory_v2.load
• since fp is corrupt, no valid tags are read. you can see this in the logs of many unknown tags
• no valid exif rotation tag means Image.transpose() is not called in load_end()
• the image/tile size is confused (WxH swapped) somewhere. causing the tile size check to fail

Workarounds

I've come up with a handful of workarounds:

1. Don't buffer

img = PIL.Image.open(img_path.open('rb', buffering=0))

The buffer can't be corrupted if there isn't one.

2. Pre-read exif tags

img = PIL.Image.open(img_path)
img.getexif()

This calls ImageFileDirectory_v2.load on a virgin fp, so it's not corrupt. Subsequent calls in Image.load are cached.

3. Poke the buffer

Add after here https://github.com/python-pillow/Pillow/blob/main/src/PIL/TiffImagePlugin.py#L1385

            n, err = decoder.decode(b"fpfp")
            if not close_self_fp:
                self.fp.seek(0, 2)
                assert self.fp.read(1) == b''
                self.fp.seek(0)

This gets the buffer to drop its cache.
I got the idea from comments and similar code here: fd299e3

However, I don't think this is a safe idea. This is not a documented behavior and the buffering/caching behavior of this could change at any time. When I tried with just seek(0), it did not help, and looking at the CPython code, it looks like seek() might be optimized if it's a location that's already buffered and won't actually move the file handle.

pillow.zip

[Knio]

Broken case: Exif.load_from_fp called after _load_libtiff

Good case:
Exif loaded before, Image.transpose gets called

Also, I think this explains my confusion in the other thread about not seeing exif tags sometimes

[radarhere]

Pillow: 11.1.0.dev0

Could you be more specific about which commit you're building from? Does this happen with the released Pillow 11.0.0?

[Knio]

Pillow: 11.1.0.dev0

Could you be more specific about which commit you're building from? Does this happen with the released Pillow 11.0.0?

I can repro on all of the following on Linux
Local builds:

• 5bff2f3b2 - (HEAD -> main, origin/main, origin/HEAD)
• 5ca487720 - (HEAD -> tiff_orientation, origin/tiff_orientation)

PIP:

• the released pillow-11.0.0-cp311-cp311-manylinux_2_28_x86_64.whl

However, on windows (now Python 3.13.0 (tags/v3.13.0:60403a5, Oct 7 2024, 09:38:07) [MSC v.1941 64 bit (AMD64)]), I cannot repro using the pip package (pillow-11.0.0-cp313-cp313-win_amd64.whl)

[radarhere]

I'm unable to reproduce your problem.

I've created https://github.com/radarhere/docker-images/tree/refs/heads/8559 containing a Dockerfile that runs

from PIL import Image
print(Image.__version__)
im = Image.open('IMG_0282.CR2')
im.save('out.jpg')

on Debian Bookworm amd64. You can see that it runs successfully at https://github.com/radarhere/docker-images/actions/runs/11884545565/job/33112887235#step:4:2119

[Knio]

Can you try with this?

from PIL import Image
print(Image.__version__)
im = Image.open(open('IMG_0282.CR2', 'rb', buffering=1048576))
im.save('out.jpg')

That's the default buffering value on my Linux system.

This test case now repros the issue even on Windows for me

[radarhere]

Yes, that change does trigger the error - https://github.com/radarhere/docker-images/actions/runs/11908096108/job/33182903863#step:4:2134

[Knio]

May I suggest this as a fix? Working on all my machines now:

diff --git a/src/PIL/TiffImagePlugin.py b/src/PIL/TiffImagePlugin.py
index 6bf39b75a..7f1d46fca 100644
--- a/src/PIL/TiffImagePlugin.py
+++ b/src/PIL/TiffImagePlugin.py
@@ -1216,10 +1216,6 @@ class TiffImageFile(ImageFile.ImageFile):
     def _seek(self, frame: int) -> None:
         self.fp = self._fp

-        # reset buffered io handle in case fp
-        # was passed to libtiff, invalidating the buffer
-        self.fp.tell()
-
         while len(self._frame_pos) <= frame:
             if not self.__next:
                 msg = "no more images in TIFF file"
@@ -1303,10 +1299,6 @@ class TiffImageFile(ImageFile.ImageFile):
         if not self.is_animated:
             self._close_exclusive_fp_after_loading = True

-            # reset buffered io handle in case fp
-            # was passed to libtiff, invalidating the buffer
-            self.fp.tell()
-
             # load IFD data from fp before it is closed
             exif = self.getexif()
             for key in TiffTags.TAGS_V2_GROUPS:
@@ -1381,8 +1373,16 @@ class TiffImageFile(ImageFile.ImageFile):
             logger.debug("have fileno, calling fileno version of the decoder.")
             if not close_self_fp:
                 self.fp.seek(0)
+            # Save and restore the file position, because libtiff will move it
+            # outside of the python runtime, and that will confuse
+            # io.BufferedReader and possible others.
+            # NOTE: This must use os.lseek, and not fp.seek(), because
+            # fp.seek() may just adjust it's internal buffer pointer and not
+            # actually move the OS file handle.
+            pos = os.lseek(fp, 0, os.SEEK_CUR)
             # 4 bytes, otherwise the trace might error out
             n, err = decoder.decode(b"fpfp")
+            os.lseek(fp, pos, os.SEEK_SET)
         else:
             # we have something else.
             logger.debug("don't have fileno or getvalue. just reading")

[radarhere]

The removed code was added in #5443 and #5575. The test suite passes without them now (even without your new code).

Would you like to create a PR with your suggestion? If it could have a test with a significantly smaller test file (or even re-using of one of our existing ones), that would be helpful.

[Knio]

Sure, I can make a PR. I'll see what I can do about a test image but mine are all pretty big

[Knio]

Pushed #8560

[radarhere]

Thanks.

That's the default buffering value on my Linux system.

I'm guessing this is a different value to normal? Could you expand on why you have a different setting, so that we could get a better picture of who this might affect?

[Knio]

From the docs

An int containing the default buffer size used by the module’s buffered I/O classes. open() uses the file’s blksize (as obtained by os.stat()) if possible.

So it looks like this could depend on the filesystem or even the individual file. I've been using ZFS or a CIFS mount, so maybe that explains the large block size.