reformat: fix u32 row offset wrap in slow path#3202
Open
rootvector2 wants to merge 1 commit into
Open
Conversation
In the slow YUV-to-RGB fallback in src/reformat.c, the Y/U/V/A row pointers were computed with 'j * yRowBytes' style multiplications where both operands are uint32_t. For images whose plane size exceeds 4 GiB the product wraps around and the loop reads from a different row of the same buffer, producing silently corrupted output. Cast the row index to size_t before the multiplication so the offset is computed in size_t arithmetic, matching what is already done for the RGB destination pointers a few lines below and the same pattern applied in c79a400.
d14b742 to
87cc17a
Compare
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
In the slow YUV-to-RGB fallback
avifImageYUVAnyToRGBAnySlow()(src/reformat.c:686-692), the per-row Y/U/V/A pointers are computed withj * yRowBytesstyle multiplications where both operands areuint32_t. When the plane size exceeds 4 GiB (e.g. a 70000x70000 8-bit YUV400 image, or any combination whoseheight * yRowBytesexceedsUINT32_MAX), the product wraps around in 32 bits before being promoted tosize_tfor the pointer arithmetic. The loop then reads from a different row inside the same plane buffer, producing silently corrupted output.The neighbouring RGB destination pointers in the same loop body already use
(size_t)j * rgb->rowBytes, and the same pattern was applied across the file in c79a400 and f17110d. This change applies the same cast to the four YUV/A reads that were missed.Reproduction
Built a small driver that constructs an
avifImagewithwidth=2, height=4, AVIF_PIXEL_FORMAT_YUV420, points the Y plane at anmmapregion (virtual, only a few pages touched physically) and setsyuvRowBytes[Y] = 0x60000000. Distinct sentinel bytes are written at offsets0,0x60000000,0xC0000000, the true row-3 offset0x120000000(0xCC), and the wrapped uint32_t row-3 offset0x20000000(0xBB).avifImageYUVToRGBis then called withAVIF_CHROMA_UPSAMPLING_BILINEARandalphaPremultiplied=AVIF_TRUEto force the slow path.Pre-patch output:
i.e. row 3 of the output mirrors the wrapped-offset sentinel, not the true row-3 sentinel.
Post-patch output:
i.e. row 3 reads from the correct offset.
Test plan
0xBB(wrapped offset) for the last row, post-patch reads0xCC(true offset).avifyuv_limited/avifyuv_rgbtests still pass (aviftestis unrelated and fails only because the local build is configured without an AV1 codec).