Fix heap-buffer-overflow write in PICT UnpackBits#47
Open
mjbommar wants to merge 1 commit into
Open
Conversation
The PackBits decompression loop in UnpackBits advances dst by the decoded run length without verifying that the destination remains within the allocated bitmap raster. A crafted PICT file with run lengths that exceed the image causes expandBuf/expandBuf8 and the repeat memcpy to write past the end of the bitmap. This is the sibling of the UnpackPictRow fix (PR danoli3#41). UnpackBits is the decoder used for 1/2/4/16-bpp PICT data (the 8/32-bpp paths go through the now-bounded UnpackPictRow), and it has the same unbounded run problem; it is reachable from both the version-1 bitmap opcode and the version-2 DirectBitsRect/PackBitsRect opcodes. Clamp each write to the allocated raster: compute the raster end once (FreeImage_GetBits(dib) + pitch * height) and break out of the row loop if a run would write past it, in both the packed-data and unpacked-data code paths. CWE-787 (Out-of-bounds Write) Assisted-by: Claude:claude-opus-4-8
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
1 participant
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.
Sibling of merged PR #41 (UnpackPictRow).
UnpackBitsinSource/FreeImage/PluginPICT.cpphas the same unbounded-run defect: its per-row PackBits loop advancesdstinto the bitmap raster by an attacker-controlled run length with no check against the end of the raster, so a crafted PICT whose row decompresses past the image writes out of bounds on the heap (reachable viaFreeImage_Load).UnpackBitsis the decoder used for 1/2/4/16-bpp PICT data; the 8/32-bpp paths already go through the boundedUnpackPictRowfixed in #41. It is reachable from both the version-1 bitmap opcode (0x98) and the version-2 DirectBitsRect/PackBitsRect opcodes (0x9a/0x98), so it triggers through normal content-sniffing format detection.This PR bounds every write in the
UnpackBitsRLE loop to the allocated raster, exactly like #41: computerasterEnd = FreeImage_GetBits(dib) + pitch * heightonce, and break out of the row loop if a run would write past it, in both the packed-data and unpacked-data code paths.Verified: a PICT that previously triggered an AddressSanitizer heap-buffer-overflow now decodes without any out-of-bounds access, and well-formed PICT images still decode.
CWE-787 (out-of-bounds heap write); denial-of-service / memory corruption (the reachable write content is constrained, so this is not code execution).