gainmapmath: restore odd-dimension OOB read fixes reverted by #413 - #459
Open
MajorMurphy wants to merge 1 commit into
Open
gainmapmath: restore odd-dimension OOB read fixes reverted by #413#459MajorMurphy wants to merge 1 commit into
MajorMurphy wants to merge 1 commit into
Conversation
…on images (google#407) * Fix OOB write in applyGainMap by validating destination image pointers, stride, output_ct, and pixel format (b/439639756) * Fix OOB write in applyGainMap by validating destination image pointers, stride, output_ct, and pixel format (b/439639756) * gainmapmath: fix OOB reads in encoder pixel processing on odd-dimension images
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
2 participants
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.
Restores the defect fix from #391, originally submitted by @jortles in #392 and landed by @DichenZhang1 as #407 (
586ed42).#413's
07bc6a2was branched before #407 and reverted all three hunks when it merged nine days later, so v2.0.0, v2.0.1, v2.0.2 and currentmainall ship the pre-fix code:lib/src/gainmapmath.cpp:432—getP010Pixel's chroma index is back to(x & ~0x1)with no clamp, sopixel_v_idx = pixel_u_idx + 1reads one element past the chroma plane on the last column of an odd-width P010 image.lib/src/gainmapmath.cpp:1320and:1413—convert_raw_input_to_ycbcr's 2×2 block loops are back toi < dst->h/j < dst->w, reading rowi + 1/ columnj + 1past the edge on odd dimensions. Both the RGBA1010102 and RGBA8888 paths.git diff 586ed42 07bc6a2 -- lib/src/gainmapmath.cppshows the three hunks being undone. The revert looks incidental rather than deliberate — the HEIF/AVIF staging branch predated #407 and carried the older copy of the file forward over it.#407's test
GetP010PixelOddWidthwas not reverted and is still intests/gainmapmath_test.cpp, but it assertsEXPECT_NO_FATAL_FAILURE, which a short read past a heap allocation does not trip outside a sanitizer build. The test and the code it guards were reverted independently, leaving a green guard over absent code.This is a cherry-pick of
586ed42onto currentmain, preserving its original authorship. The test hunk is already present, so onlylib/src/gainmapmath.cppchanges (+9 / −5).One caveat carried over from #407: the loop-bound half skips the trailing incomplete block rather than clamping, so the final row and column of an odd-dimension image are left value-initialized — black with fully saturated chroma — instead of read out of bounds. Clamping the source index the way
getP010Pixelnow does would fix the values too, but I have kept this to exactly what #407 did so it reads as a pure restoration. Happy to follow up.Verified to build clean on MSVC 2022 x64 Release; I have not run it under ASan.