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

uint256 types can be uint64 #58

Open
code423n4 opened this issue Dec 12, 2021 · 0 comments
Open

uint256 types can be uint64 #58

code423n4 opened this issue Dec 12, 2021 · 0 comments
Assignees
Labels
bug Something isn't working G (Gas Optimization) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

Handle

sirhashalot

Vulnerability details

Impact

The _calculateRewardAmount() function in TwabRewards.sol uses the uint256 type for the three variables _epochDuration, _epochStartTimestamp, and _epochEndTimestamp. However, there is no need for these variable to be uint256 instead of uint64 because 1. these variables are later cast as uint64 values anyway 2. the block.timestamp value is orders of magnitude less than the uint64 max value. To expand on this second point, if the the block.timestamp values were on the same order of magnitude as the uint64 max value, then the casting of the uint256 timestamp values to uint64 could cause overflow issues because the OpenZeppelin SafeCast library is not used.

The timestamp values could even be of type uint32 (Uniswap v3 does this in places, and the max uint32 timestamp equates to the year 2106), but since the ITicket.sol contract imported by TwabRewards.sol uses uint64, it would be better to use uint64 to maintain consistency.

Proof of Concept

The uint256 variables that can be uint64 are found in TwabRewards.sol:
https://github.com/pooltogether/v4-periphery/blob/b520faea26bcf60371012f6cb246aa149abd3c7d/contracts/TwabRewards.sol#L294-L296

Tools Used

Manual analysis

Recommended Mitigation Steps

Make these variables uint64 for gas savings and consistency with Iticket.sol timestamps. Remove unnecessary uint64() casting when all variables in the _calculateRewardAmount() function consistently use uint64 types.

@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Dec 12, 2021
code423n4 added a commit that referenced this issue Dec 12, 2021
@PierrickGT PierrickGT self-assigned this Dec 14, 2021
@PierrickGT PierrickGT added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working G (Gas Optimization) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

2 participants