Fix unpack to c_ssize_t on 32 bit systems with corrupt EXIF data

[paddywwoof]

Add an additional specific try except to allow continued iteration through tags and image data to be subsequently read. On 64bit systems this isn't an issue as 4 bytes can't be unpacked to overflow ssize_t.

The alternative approach would have been to mask the value of offset like:

offset, = self._unpack("L", data)
offset = offset & ((1 < 31) - 1)

but that would prevent offset ever taking a negative value which might be bad

[radarhere]

Would you be able to add a test? One that fails without your fix and passes with it.

[paddywwoof]

Impressive amount of checking, if maybe a little pedantic! However I'm not sure how the issue targeted by this pull request will be verified as it needs to be run on a 32 bit system. Is this done manually prior to each release?

Paddy

[radarhere]

Half of the AppVeyor jobs are 32-bit, so it should already be tested.

[paddywwoof]

Great. I'm not sure what the appveyor test failure is (other pull requests also seem to be suffering from it) - presumably relates to #4019. Let me know if there's anything I can do to help or test.

[hugovk]

#4019 is now merged, if you rebase or merge from master it should now pass.

[radarhere]

If I print out the tags and the offsets at the relevant line,

diff --git a/src/PIL/TiffImagePlugin.py b/src/PIL/TiffImagePlugin.py
index dabcf8eb4..c6009a036 100644
--- a/src/PIL/TiffImagePlugin.py
+++ b/src/PIL/TiffImagePlugin.py
@@ -771,6 +771,7 @@ class ImageFileDirectory_v2(MutableMapping):
                             "Tag Location: %s - Data Location: %s" % (here, offset),
                             end=" ",
                         )
+                    print("Tag "+str(tag)+": Offset "+str(offset))
                     fp.seek(offset)
                     data = ImageFile._safe_read(fp, size)
                     fp.seek(here)

I get

Tag 271: Offset 8
Tag 272: Offset 26
Tag 282: Offset 38
Tag 283: Offset 46
Tag 305: Offset 54
Tag 306: Offset 96
Tag 33432: Offset 116
Tag 33434: Offset 138
Tag 33437: Offset 146
Tag 36867: Offset 154
Tag 36868: Offset 174
Tag 37377: Offset 194
Tag 37378: Offset 202
Tag 37380: Offset 210
Tag 37381: Offset 218
Tag 37386: Offset 226
Tag 41988: Offset 234
Tag 42033: Offset 242
Tag 42036: Offset 250
Tag 2: Offset 280
Tag 4: Offset 304

This looks like a problem with the test image to me - at least one of the offset values should be greater than 32-bit to trigger the error, yes?

[paddywwoof]

Andrew, this is my mistake. The latest version of Pillow does generate the tags you list, the version I tested the python mods on produced these tags from the same file:

Tag 0: Offset 65536
Tag 0: Offset 268493055
Tag 4352: Offset 33563137
Tag 221: Offset 66316
Tag 55807: Offset 18
Tag 5: Offset 2191327232
...

If the fixes to the exif reading and associated tests can guarantee that no big offsets will get through then my modification is superfluous and can be rejected.

[radarhere]

What version of Pillow are you testing with?

[radarhere]

Testing, I find that this problem - #3994 - is fixed by #3584, in Pillow 6.0. See also the issue behind that PR, #3337

So, your image can be read correctly by current Pillow. However, I think it's because the image is actually fine - meaning that the fix does not address any concerns you might have about 32-bit systems.

[paddywwoof]

@radarhere , yes. The version I was using and testing on was the 'standard' Pillow packaged with debian buster as raspbian for Raspberry Pi (still version 5.1). I intend the module I maintain (pi3d) to be easy for user of the vanilla system without having to upgrade PIL so I was reluctant to do that on the system I was testing on. However on a new setup I have tested the latest PIL and, as you say, the #3584 fix has prevented the corrupt offsets that caused the problem. If the exif corruption isn't going to happen in future then my suggested mod is not needed and I'm happy for it to be nullified.

Do I need to do that or can you, as a Pillow member?

[radarhere]

I can do that. Thanks for your help in discussing the problem