Raise ValueError for truncated gAMA and cHRM PNG chunks

[VenishPaneliya]

Summary

chunk_gAMA reads the 4-byte gamma value with i32(s) without first checking the chunk length. A PNG whose gAMA chunk is shorter than 4 bytes raises struct.error, which ImageFile.__init__ converts to SyntaxError, which Image.open() treats as "this isn't a PNG" — so the user sees UnidentifiedImageError: cannot identify image file for a file that is otherwise a perfectly readable PNG.

LOAD_TRUNCATED_IMAGES can't rescue it either, because the failure happens before any truncation handling runs.

The sibling chunk handlers already do this correctly. IHDR, sRGB, pHYs, acTL, fcTL and fdAT all check the length first and either return the short chunk when LOAD_TRUNCATED_IMAGES is set, or raise ValueError("Truncated <cid> chunk"). gAMA is the one fixed-size chunk missing that guard — and it's also the one missing from the existing test_truncated_chunks parametrisation.

Reproduction

import io, struct, zlib
from PIL import Image, ImageFile

def chunk(tag, data):
    return (struct.pack(">I", len(data)) + tag + data
            + struct.pack(">I", zlib.crc32(tag + data) & 0xFFFFFFFF))

ihdr = struct.pack(">IIBBBBB", 1, 1, 8, 0, 0, 0, 0)
data = (b"\x89PNG\r\n\x1a\n" + chunk(b"IHDR", ihdr)
        + chunk(b"gAMA", b"\x00\x01")                     # 2 bytes instead of 4
        + chunk(b"IDAT", zlib.compress(b"\x00\x00")) + chunk(b"IEND", b""))

ImageFile.LOAD_TRUNCATED_IMAGES = True
Image.open(io.BytesIO(data)).load()

Before:

UnidentifiedImageError: cannot identify image file <_io.BytesIO object ...>

(underlying cause: struct.error: unpack_from requires a buffer of at least 4 bytes)

After, matching sRGB exactly:

• LOAD_TRUNCATED_IMAGES = False → ValueError: Truncated gAMA chunk
• LOAD_TRUNCATED_IMAGES = True → loads fine

Changes

         s = ImageFile._safe_read(self.fp, length)
+        if length < 4:
+            if ImageFile.LOAD_TRUNCATED_IMAGES:
+                return s
+            msg = "Truncated gAMA chunk"
+            raise ValueError(msg)
         self.im_info["gamma"] = i32(s) / 100000.0

and b"gAMA" added to the existing test_truncated_chunks parametrisation, which already covers the other guarded chunks.

Tests

Tests/test_file_png.py::TestFilePng::test_truncated_chunks[gAMA] fails on current main with struct.error and passes with the guard. Full Tests/test_file_png.py: 65 passed, 1 skipped (the skip is Unix-only). ruff and black clean on both changed files.

One question

chunk_cHRM has the same crash — struct.unpack(f">{len(s) // 4}I", s) raises struct.error when the length isn't a multiple of 4. I left it out of this PR because the right guard there is a judgement call: strict (length < 32, per spec) would reject short-but-parseable chromaticity chunks that currently decode into a partial tuple. Happy to add whichever you prefer, here or separately.

[radarhere]

Hi. Thanks for this. Curious question - did you actually find an image in the wild where this was truncated, or is this a theoretical concern?

Regarding cHRM, I think check for 32 length. Looking at the other chunks that check for length, they are checking for the full specified length. Feel free to add it in this PR.

[VenishPaneliya]

Theoretical — I haven't got a real-world file for this. I found it by comparing the chunk handlers against each other: IHDR, sRGB, pHYs, acTL, fcTL and fdAT all validate their length, and gAMA/cHRM were the two fixed-size chunks that didn't. The PNGs in the description are hand-built to hit the paths.

So it's a robustness/consistency fix rather than a response to a file someone hit in the wild. The part I'd argue is worth having is that LOAD_TRUNCATED_IMAGES = True currently can't rescue these — the struct.error fires before any truncation handling — and the resulting UnidentifiedImageError: cannot identify image file points away from the real cause, which is one bad ancillary chunk in an otherwise readable PNG. Truncated files do turn up from interrupted downloads and partial writes, which is what that flag exists for. Happy for you to judge whether that clears the bar.

cHRM is added in cc1602b, checking for the full 32 bytes as you suggested.

One thing I ran into while doing it: the length check alone doesn't cover everything, because the existing unpack derives its format from the buffer size:

raw_vals = struct.unpack(f">{len(s) // 4}I", s)

With length = 33 that builds ">8I", which wants 32 bytes while s holds 33, so struct.unpack still raises struct.error — the same failure the PR is fixing, just above the threshold instead of below it. And length = 36 quietly yields 9 chromaticity values rather than 8.

So I unpack exactly 8 ints from the first 32 bytes:

raw_vals = struct.unpack(">8I", s[:32])

Behaviour now: 32 and 33 and 36 bytes all give the same 8 values, and anything under 32 raises ValueError("Truncated cHRM chunk") (or returns the short chunk under LOAD_TRUNCATED_IMAGES).

That does change one existing behaviour: an over-length cHRM used to return the extra values, and now they're ignored. Since the chunk is specified as exactly 8 unsigned ints I think ignoring them is right, but say the word and I'll drop that half and leave just the length < 32 check.

cHRM is also added to the test_truncated_chunks parametrisation, so it now covers all eight guarded chunks. Tests/test_file_png.py: 66 passed, 1 skipped.