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

Use unchecked{} where the underlying arithmetic can be determined to not overflow/underflow
 #152

Closed
code423n4 opened this issue Sep 22, 2021 · 2 comments
Labels
bug Warden finding duplicate Another warden found this issue G (Gas Optimization) sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons

Comments

@code423n4
Copy link
Contributor

Handle

0xRajeev

Vulnerability details

Impact

Using unchecked{} where the underlying arithmetic can be determined to not overflow/underflow will save gas from skipping the built-in Solidity checks of compiler version >= 0.8.0.

Example 1: based on the control flow, we can determine that lastFee will always be less than block.timestamp and therefore we can use unchecked{} for their subtraction on Basket:L116 because there is no risk of underflow.

Example 2: Given the mints, we can be assured that totalSupply() >= startSupply and therefore the RHS on L124 will be <= ibRatio and therefore we can use unchecked{} for the calculation.

Proof of Concept

https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Basket.sol#L116

https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Basket.sol#L124

Tools Used

Manual Analysis

Recommended Mitigation Steps

Use unchecked{} where the underlying arithmetic can be determined to not overflow/underflow

code423n4 added a commit that referenced this issue Sep 22, 2021
@frank-beard frank-beard added the sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons label Nov 6, 2021
@GalloDaSballo
Copy link
Collaborator

Agree with finding

@GalloDaSballo
Copy link
Collaborator

Duplicate of #135

@GalloDaSballo GalloDaSballo marked this as a duplicate of #135 Nov 30, 2021
@GalloDaSballo GalloDaSballo added the duplicate Another warden found this issue label Nov 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Warden finding duplicate Another warden found this issue G (Gas Optimization) sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons
Projects
None yet
Development

No branches or pull requests

3 participants