Fix unchecked integer overflow in encoder UV plane allocation#3233
Open
jortles wants to merge 1 commit into
Open
Fix unchecked integer overflow in encoder UV plane allocation#3233jortles wants to merge 1 commit into
jortles wants to merge 1 commit into
Conversation
Add pre-multiplication overflow checks to codec_avm.c and codec_svt.c for UV plane size computations, consistent with the existing safe pattern in avifImageAllocatePlanes() (avif.c:435-441). Also add an overflow check in avifImageCopyProperties() before the numProperties allocation, consistent with avifImagePushProperty() (avif.c:387). Without these checks, crafted image dimensions can silently wrap the uint32_t multiplication, leading to undersized allocations. Signed-off-by: Anthony Hurtado <amhurtado@pm.me>
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.
Summary
codec_avm.candcodec_svt.cfor UV plane size computations, consistent with the existing safe pattern inavifImageAllocatePlanes()(avif.c:435-441)avifImageCopyProperties()beforenumPropertiesallocation, consistent withavifImagePushProperty()(avif.c:387)Details
Three sites compute allocation sizes via unchecked integer multiplication, unlike their parallel functions which validate before multiplying:
codec_avm.c:919 —
channelSize * monoUVWidth(uint32_t) andmonoUVHeight * monoUVRowBytes(size_t) used foravifAlloc()without overflow checks. The parallel code inavifImageAllocatePlanes()checkswidth > UINT32_MAX / channelSizeandheight > PTRDIFF_MAX / fullRowBytesbefore the same operations.codec_svt.c:286 —
uvWidth * bytesPerPixel(uint32_t) anduvHeight * uvRowBytes(size_t) computed without pre-multiplication validation. The existing post-hoc check (uvSize > UINT32_MAX / 2) cannot detect a wrap that already occurred.avif.c:237 —
numProperties * sizeof(...)passed toavifAlloc()without an overflow guard.avifImagePushProperty()in the same file already checksnumProperties < SIZE_MAX / sizeof(avifImageItemProperty)before an identical allocation.Test plan
cmake --build . --target avif_obj(zero warnings)