n1punp - FeeGrowthInside calculation doesn't allow overflowing/underflowing

[sherlock-admin]

n1punp

high

FeeGrowthInside calculation doesn't allow overflowing/underflowing

Summary

When calculating feeGrowthInside , the calculation doesn't allow overflowing/underflowing.

Vulnerability Detail

When calculating feeGrowthInside , the calculation doesn't allow overflowing/underflowing. See more details in Uniswap/v3-core#573

Impact

Calculation can revert in some situations, causing the transactions to revert, including deposit function.

Code Snippet

https://github.com/sherlock-audit/2023-06-real-wagmi/blob/main/concentrator/contracts/Multipool.sol#L668-L690

Tool used

Manual Review

Recommendation

• Use unchecked in the calculation to allow under/overflow.

Duplicate of #46

[ctf-sec]

Valid medium based on Uniswap/v3-core#573

[sherlock-admin]

Escalate

Medium severity is okay, per @ctf-sec . But this should be a valid issue.

You've deleted an escalation for this issue.

[n1punp]

Escalate

Same as #46

[sherlock-admin]

Escalate

Same as #46

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[ctf-sec]

Agree with escalation, this is a duplicate of #46

[hrishibhat]

Result:
Medium
Duplicate of #46

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• n1punp: accepted

[fann95]

fixed: https://github.com/RealWagmi/concentrator/blob/9036fe43b0de2694a0c5e18cfe821d40ce17a9b8/contracts/Multipool.sol#L675

[0xffff11]

Fixed by adding unchecked several times to all the calculations to allow under/overflowing as it is intended for uniswap libs. Also, instead of using that many unchecked, sponsor could use one, but scope all the code. Regardless, it is also fixed this way.