Skip to content

Conversation

@lewismosciski
Copy link

fix an issue #726 (comment)

@cospectrum
Copy link
Contributor

cospectrum commented Oct 21, 2025

I think assertion about height + padding - 1 < buffer.len()
can be replaced with single assert!(height.checked_add(padding).unwrap() - 1 < buffer.len())

@lewismosciski
Copy link
Author

Hi @cospectrum , thanks for the review.

You're right that your suggested check (height.checked_add(padding)) would fix the safety issue in the unsafe block.

However, the function's logic relies on assertion 1 to validate the total space for all three loops—including the third loop (buffer[padding + height..]).

for b in &mut buffer[padding as usize + height as usize..] {
sum += last;
*b = sum;

The root problem is that the height + 2 * padding calculation inside assertion 1 is what overflows. Your suggested check doesn't fix that; assertion 1 would still be broken.

My PR's assertion 4 is intended to protect assertion 1 from overflowing, so that it can safely perform the correct height + 2 * padding check. This fixes the soundness vulnerability without breaking the function's intended logic.

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.

2 participants