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 FuzzedDataProvider crash on float length #552

Merged
merged 4 commits into from
Aug 16, 2023

Conversation

br-lewis
Copy link
Contributor

This adds checks to all functions in FuzzedDataProvider that accept a parameter that could be used as the number of bytes to pull from the fuzzed data. If any length value is not a whole number, the function will throw an error to prevent dataPtr from ending up a non-whole number. I chose this over having it do a Math.floor and continuing to make clear what's going on.

The crashes we were seeing were due to consumeNumberInRange returning a non-integer value, consumeString would accept that value as the length of the string to get and would set this.dataPtr and this._remainingBytes as a direct calculation based off of the length given. I'm not sure what this.data.toString does with the floating value but it successfully returned something so this call would work. The next call to consumeNumberInRange would end up trying to index this.data with this.dataPtr + <some other number> and the end result was this.data[<a non-integer value>] which JS will deal with by returning undefined.

@br-lewis br-lewis requested a review from a team August 14, 2023 14:36
packages/core/FuzzedDataProvider.ts Show resolved Hide resolved
packages/core/FuzzedDataProvider.ts Outdated Show resolved Hide resolved
packages/core/FuzzedDataProvider.ts Show resolved Hide resolved
packages/core/FuzzedDataProvider.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@bertschneider bertschneider left a comment

Choose a reason for hiding this comment

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

Looks good! 👍

Could you please squash the three commits together, as they belong to one functional change.

@br-lewis br-lewis merged commit 4082ad9 into main Aug 16, 2023
9 checks passed
@br-lewis br-lewis deleted the FUZZ-781-dataprovider-crash branch August 16, 2023 08:48
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.

None yet

2 participants