Repository navigation
paint: fix an out-of-bounds write crash in transform:rotate() - #263
Merged
Merged
Conversation
paintRotated sized its offscreen buffer using only the rotated box's own border box, trusting it to bound everything paintBoxContent's recursion could write there. A descendant (a shrink-wrapped float, a negative margin, a marker, an outset box-shadow) can paint below the box's own bottom edge, and blitImage/blendPixel have no bound beyond the ancestor clip, which is not scoped to the buffer's own smaller height. The write went past the buffer's allocation and crashed the render. Found by a 500-page bench sweep (round 165), reproduced directly against the live page with no bench harness involved: https://www.smashingmagazine.com/2018/10/mobile-app-retention-rate/ Fixed by sizing the buffer from subtreeExtent (the filter/opacity/ mask-image group path's own ink-bounds helper), which already covers the box's own border box unconditionally, so the result is never shorter than before. Stash-verified: reverting paint.go alone reproduces the real panic with the identical paintBox -> paintBoxContent -> blendPixel stack. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
A 500-page corpus sweep (268 sites, round 165) crashed the renderer on page 21, a real site, with no bench harness involved — reproduced directly against the live URL.
Root cause
paintRotatedsized its offscreen buffer using only the rotated box's own border box, trusting it to bound everything the paint recursion could write. A descendant (shrink-wrapped float, negative margin, marker, outset box-shadow) can paint below the box's own bottom edge, andblitImage/blendPixelhave no bound beyond the ancestor clip — which is not scoped to the buffer's own smaller height. The write went past the allocation and crashed the render.Fix
The buffer height now comes from
subtreeExtent— the existing filter/opacity/mask-image group path's own ink-bounds helper — which already covers the box's full border box unconditionally, so the result is provably never shorter than before. The final rotated image's own disclosed scope (content outside the border box is not captured into what gets rotated) is unchanged; only the offscreen buffer's own allocation grows.Verification
TestRotateDoesNotCrashWhenAChildOverflowsTheBoxBottom: stash-verified against the real unfixed code, reproducing the identicalpaintBox -> paintBoxContent -> blendPixelpanic.paintcoverage briefly dropped to 99.9% on a clamp branch that turned out to be unreachable by construction; deleted rather than tested around, restoring the 100% floor.go vet, and all five package coverage floors green.🤖 Generated with Claude Code