Skip to content

clamp frame delay repeat count in SetAnimationProperties - #4618

Open
metsw24-max wants to merge 1 commit into
lovell:mainfrom
metsw24-max:set-animation-delay-count
Open

metsw24-max wants to merge 1 commit into
lovell:mainfrom
metsw24-max:set-animation-delay-count

Conversation

@metsw24-max

Copy link
Copy Markdown
Contributor

Out-of-bounds write from an underflowed frame count

SetAnimationProperties repeats a single delay entry with delay.insert(delay.end(), nPages - 1, delay[0]), and because that count is a size_type a resolved page count of zero or less turns it into a near-SIZE_MAX value, after which the fill runs well past the end of the allocation. Two routes reach it: pages: 0 is accepted by is.inRange(pages, -1, 100000) and arrives as-is, and with animated set the count becomes the input's n-pages minus the requested page, which goes negative for input whose loader does not validate the page number, so I think the guard belongs in the native helper rather than the JS validation.

Repro, which dies with EXC_BAD_ACCESS (code=2) on the store in the fill loop (same for webp({ delay })):

await sharp(input, { pages: 0 }).gif({ delay: 100 }).toBuffer();

The clamp is a no-op for any page count of one or more. Regression test added to test/unit/gif.js; it takes the test process down with SIGTRAP before the change, and the full suite is green after.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant