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

Division by 0 #232

Closed
code423n4 opened this issue Jun 24, 2022 · 1 comment
Closed

Division by 0 #232

code423n4 opened this issue Jun 24, 2022 · 1 comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working invalid This doesn't seem right sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/Test/UpgradedNibblVault.sol#L484

Vulnerability details

Division by 0 can lead to accidentally revert,
(An example of a similar issue - code-423n4/2021-10-defiprotocol-findings#84)

Code instances:

    https://github.com/code-423n4/2022-06-nibbl/tree/main/contracts/NibblVault.sol#L183 _initialTokenSupply, _initialTokenPrice might be 0
@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Jun 24, 2022
code423n4 added a commit that referenced this issue Jun 24, 2022
@mundhrakeshav mundhrakeshav added the sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue label Jun 26, 2022
@HardlyDifficult
Copy link
Collaborator

The link provided is incorrect. However the instance listed has the correct source for this report.

Although technically correct, the issue occurs on the first line of the initialize function. If 0 was provided for one of the fields mentioned then the transaction reverts and it could be retried with correct inputs.

There's no recommendation provided here. Adding a require statement for example would have the same effect, only providing a more clear revert reason.

@HardlyDifficult HardlyDifficult added the invalid This doesn't seem right label Jun 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working invalid This doesn't seem right sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue
Projects
None yet
Development

No branches or pull requests

3 participants