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

getFeeGrowthInside can incorrectly return zero because of fee growth underflow #26

Closed
c4-bot-1 opened this issue Oct 11, 2024 · 0 comments
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 insufficient quality report This report is not of sufficient quality 🤖_primary AI based primary recommendation

Comments

@c4-bot-1
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2024-10-ramses-exchange/blob/236e9e9e0cf452828ab82620b6c36c1e6c7bb441/contracts/CL/core/libraries/Tick.sol#L40-L77

Vulnerability details

Proof of Concept

When operations need to calculate Uniswap V3 position's fee growth, it used similar function implemented by uniswap v3.
However, according to this known issue, The contract is implicitly relies on underflow/overflow when calculating the fee growth, if underflow is prevented, some operations that rely on fee growth will revert.

function getFeeGrowthInside(
        mapping(int24 => TickInfo) storage self,
        int24 tickLower,
        int24 tickUpper,
        int24 tickCurrent,
        uint256 feeGrowthGlobal0X128,
        uint256 feeGrowthGlobal1X128
    ) internal view returns (uint256 feeGrowthInside0X128, uint256 feeGrowthInside1X128) {
        unchecked {
            TickInfo storage lower = self[tickLower];
            TickInfo storage upper = self[tickUpper];

            /// @dev calculate fee growth below
            uint256 feeGrowthBelow0X128;
            uint256 feeGrowthBelow1X128;
            if (tickCurrent >= tickLower) {
                feeGrowthBelow0X128 = lower.feeGrowthOutside0X128;
                feeGrowthBelow1X128 = lower.feeGrowthOutside1X128;
            } else {
                feeGrowthBelow0X128 = feeGrowthGlobal0X128 - lower.feeGrowthOutside0X128;
                feeGrowthBelow1X128 = feeGrowthGlobal1X128 - lower.feeGrowthOutside1X128;
            }

            /// @dev calculate fee growth above
            uint256 feeGrowthAbove0X128;
            uint256 feeGrowthAbove1X128;
            if (tickCurrent < tickUpper) {
                feeGrowthAbove0X128 = upper.feeGrowthOutside0X128;
                feeGrowthAbove1X128 = upper.feeGrowthOutside1X128;
            } else {
                feeGrowthAbove0X128 = feeGrowthGlobal0X128 - upper.feeGrowthOutside0X128;
                feeGrowthAbove1X128 = feeGrowthGlobal1X128 - upper.feeGrowthOutside1X128;
            }

            feeGrowthInside0X128 = feeGrowthGlobal0X128 - feeGrowthBelow0X128 - feeGrowthAbove0X128;
            feeGrowthInside1X128 = feeGrowthGlobal1X128 - feeGrowthBelow1X128 - feeGrowthAbove1X128;
        }
    }

This could impact crucial operation that rely on this call, such as liquidation, could revert unexpectedly. This behavior is quite often especially for pools that use lower fee.

Impact

some user transactions will revert.

Recommended Mitigation Steps

Use unchecked when calculating feeGrowthInside0X128 and feeGrowthInside1X128.

Assessed type

Under/Overflow

@c4-bot-1 c4-bot-1 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 Oct 11, 2024
c4-bot-5 added a commit that referenced this issue Oct 11, 2024
@c4-bot-12 c4-bot-12 added 🤖_11_group 🤖_primary AI based primary recommendation labels Oct 11, 2024
@howlbot-integration howlbot-integration bot added the insufficient quality report This report is not of sufficient quality label Oct 13, 2024
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 insufficient quality report This report is not of sufficient quality 🤖_primary AI based primary recommendation
Projects
None yet
Development

No branches or pull requests

2 participants