Fix heap overflow and underflow in EXR decoder#214
Open
sharadboni wants to merge 1 commit intogoogle:mainfrom
Open
Fix heap overflow and underflow in EXR decoder#214sharadboni wants to merge 1 commit intogoogle:mainfrom
sharadboni wants to merge 1 commit intogoogle:mainfrom
Conversation
Two bounds-safety fixes in DecodeImageEXR: 1. Heap overflow (exr.cc:365): when the EXR data window and display window have no horizontal overlap, exr_x2 < exr_x1 and the expression (exr_x2 - exr_x1 + 1) wraps to a very large size_t, causing memcpy to read and write far beyond the allocated buffer. Guard the copy block with `if (exr_x1 <= exr_x2)` so it is skipped when the windows do not overlap. 2. Heap underflow write (exr.cc:332-334): the virtual base pointer for extra-channel framebuffer slices was computed once using the aggregate extraPixelBytes stride and then shared across all extra channels. When dataWindow.min.x > 0 and there are multiple extra channels with different per-channel sizes, the shared offset calculation walks the base pointer before the start of the allocation. Fix by computing an independent ch_base_ptr for each channel using that channel's own size as the stride.
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.
Summary
Two memory-safety fixes in
lib/extras/dec/exr.cc(DecodeImageEXR):Heap overflow — non-overlapping data/display windows (
exr.cc:365)When the EXR data window and display window have no horizontal overlap,
exr_x1 = max(dataWindow.min.x, displayWindow.min.x)andexr_x2 = min(dataWindow.max.x, displayWindow.max.x)satisfyexr_x2 < exr_x1.The expression
(exr_x2 - exr_x1 + 1)is then negative, which wraps to avery large
size_t, and the subsequentmemcpyreads and writes far beyondthe allocated buffer.
Fix: guard the copy block with
if (exr_x1 <= exr_x2)so it is skipped whenthe windows have no horizontal overlap.
Heap underflow write — extra-channel framebuffer stride mismatch (
exr.cc:332–334)The virtual base pointer for extra-channel framebuffer slices was computed
once using the aggregate
extraPixelBytesstride (sum of all extra-channelsizes) and then shared across every channel's
OpenEXR::Slice. WhendataWindow.min.x > 0and there are multiple extra channels whose individualsizes differ from
extraPixelBytes, the shared offset walks the pointer beforethe start of the allocation.
Fix: compute an independent
ch_base_ptrfor each channel using thatchannel's own per-element
sizeas the stride, matching the per-channelstride passed to
Slice.