Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix panic on overflowing ascii85 decode #233

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Orycterope
Copy link
Contributor

@Orycterope Orycterope commented Aug 2, 2024

Found this bug while fuzzing the crate

The range of values that an ascii85 character can take is slightly bigger than what fits in a u32. If a, b, c, d, and e are very high, the multiplication will result in a number that cannot be represented as a u32. This should be considered as a decoding error, but the code didn't check for this case and panicked.

Do the computation in a u64 instead, and try to cast it to a u32 at the very end, returning None if the value is invalid.

Found this bug while fuzzing the crate

The range of values that an ascii85 character can take is slightly bigger than
what fits in a u32. If a, b, c, d, and e are very high, the multiplication will
result in a number that cannot be represented as a u32. This should be considered
as a decoding error, but the code didn't check for this case and panicked.

Do the computation in a u64 instead, and try to cast it to a u32 at the very end,
returning None if the value is invalid.
Copy link
Contributor

@itsbalamurali itsbalamurali left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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