Repository navigation
Reject TIFFs whose unpacker row is wider than libtiff's decoded row - #10201
Merged
Merged
Conversation
…n _decodeTile()/_decodeAsRGBA() _decodeTile() and _decodeAsRGBA() in src/libImaging/TiffDecode.c size their decode buffer from libtiff's own view of the pixel layout (TIFFTileSize()/TIFFTileRowSize(), or img.width * 4), but unpack each row with the shuffler that TiffImagePlugin._setup() picked from Pillow's separate parse of the IFD. Neither function checks that the unpacker's input row fits inside the row libtiff actually produced. _decodeStrip() has exactly that check (added for OSS-Fuzz crash-0da0 in 8.2.0); the tile and RGBA paths never got it. The two parsers can be made to disagree with a duplicated or miscounted PhotometricInterpretation/BitsPerSample tag: libtiff keeps the first occurrence (or ignores a badly-typed tag), Pillow's ImageFileDirectory_v2.load() keeps the last. Pillow then picks a wider rawmode (e.g. RGBX, RGB;16N) than libtiff actually decoded, so the unpacker reads past the end of the reused tile/RGBA buffer -- the last rows of every tile, scaling with the attacker-chosen TileWidth. A wide enough tile turns the over-read into a segfault instead of disclosure. Confirmed live: opening the report's 16x16 tiled TIFF (single 8-bit sample per pixel, but PhotometricInterpretation written as SHORT with duplicate value so Pillow selects rawmode RGBX/4 bytes-per-pixel while libtiff decodes 1 byte-per-pixel) succeeds with no error on the unfixed baseline, and rows 13-15 of the decoded image contain bytes that are not derived from the 256-byte tile buffer's real content (trailing zero/pattern runs inconsistent with the deflate-decoded image data). The fixed build rejects the same file cleanly with `OSError: decoder error -2`, the same failure mode _decodeStrip() already gives for the equivalent strip-based file. Fix (reporter's patch, applied as-is): - _decodeTile(): compute unpacker_row_byte_size in a tsize_t-widened product (avoiding the 32-bit overflow the report also flags for very wide tiles) and reuse it for the existing TIFFTileSize() sanity check; fail with IMAGING_CODEC_BROKEN when it exceeds TIFFTileRowSize(), mirroring _decodeStrip()'s guard. - _decodeAsRGBA(): fail with IMAGING_CODEC_BROKEN when state->bits > 32, since TIFFRGBAImage rows are always 32 bits/pixel, before any allocation or TIFFRGBAImageBegin() call. Adds test_unpacker_row_wider_than_libtiff_row to Tests/test_file_libtiff.py, embedding the report's count-2 tile and duplicated-tag YCbCr strip reproducers as base64 and asserting both now fail load() with "decoder error -2". Reported-by: carlini Fixes: GHSA-c6hw-5grf-ff96 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Andrew Murray <3112309+radarhere@users.noreply.github.com>
Covers the PackBits, RGBAXX case reported in GHSA-c98p-g9r2-r642, where the unpacker reads 6 bytes per pixel from libtiff's 4 byte RGBA rows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # src/libImaging/TiffDecode.c
Member
|
I've created #10210 to stop Pillow and libtiff disagreeing in the first place. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tile and RGBA TIFF decoding size their buffers from libtiff's view of the pixel layout, but unpack rows using Pillow's separately parsed rawmode. If the two disagree, the unpacker could read past the end of the buffer.
_decodeStrip()already guards against this; this adds the same row size check to_decodeTile(), and requires 32 bits per pixel in_decodeAsRGBA(), sinceTIFFRGBAImagealways produces 32-bit pixels.🤖 Generated with Claude Code