Canto Liquidity Mining Protocol - 0xDING99YA's results

Lines of code

https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/mixins/LiquidityMining.sol#L67-L154

Vulnerability details

Impact

accrueConcentratedPositionTimeWeightedLiquidity() will iterate every single tick of a user's position. Since that total tick number can be large, this function can encounter a out of gas issue and users may not be able to claim the rewards properly.

Proof of Concept

We can take a look at the for loop in accrueConcentratedPositionTimeWeightedLiquidity():

for (int24 i = lowerTick + 10; i <= upperTick - 10; ++i) {
            uint32 tickTrackingIndex = tickTrackingIndexAccruedUpTo_[poolIdx][posKey][i];
            uint32 origIndex = tickTrackingIndex;
            uint32 numTickTracking = uint32(tickTracking_[poolIdx][i].length);
            uint32 time = lastAccrued;

We can see that for each of the single tick it will performs at least 2 storage load operations (reading tickTrackingAccruedUpTo_ mapping and tickTracking_ mapping), and both SLOAD are cold storage read and will cost lots of gas. Now we consider the total tick number of a user's position, assuming a user provides range between price [2.718 (tick = 10000), 54.587 (tick = 40000)], this is a reasonable price range of user liquidity position. Combined those this function can easily cost multi millions of gas.

What's worse, if tickTracking_[poolIdx[[I].length is not zero, and a user hasn't called this function for a while and during that time the tick i is crossed lots of time, another large cost of gas will be added.

So in conclusion, the inefficiency may result in the out of gas and the function cannot work properly. The scenario mentioned above seems normal and not in extreme condition, and accrueConcentratedPositionTimeWeightedLiquidity() function will be called when claiming rewards, so I think it can be a high issue.

Tools Used

Manual Review

Recommended Mitigation Steps

Instead of iterate over every single tick of a user position, allowing the user to provide a custom lower tick and upper tick range to update their time-weighted liquidity and checks if that range is a subset of the users liquidity position. In that case user can avoid out of gas error.

Assessed type

DoS

[L-1] Time update in accrueConcentratedPositionTimeWeightedLiquidity() can be inaccurate

Impact

Time updates in accrueConcentratedPositionTimeWeightedLiquidity() is inaccurate in certain conditions, this may cause some unexpected result like one more run of a loop

Proof of Concept

https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/mixins/LiquidityMining.sol#L119-L124

if (tickTracking.exitTimestamp < nextWeek) {
    // Exit was in this week, continue with next tick
    tickActiveEnd = tickTracking.exitTimestamp;
    tickTrackingIndex++;
    dt = tickActiveEnd - tickActiveStart;
 }

In this if block, the dt is updated as tickActiveEnd - tickActiveStart, and dt will be later added to "time".

time += dt;

This is meant to update time to the tickActiveEnd, however, since tickActiveStart is not always equal to time, time may be updated to a value less than tickActiveEnd. For example, tickTracking.enterTimestamp = 90 and time = 10, tickTracking.exitTimestamp = 100, nextWeek = 604800, in this case since 100 < 604800, dt will be 100 - 90 = 10, and time will be 10 + 10 = 20, while it should be updated to 100, which is the exitTimestamp.

In the next run within the loop the time will be updated, however, that will cost one more run.

Tools Used

Manual Review

Recommended Mitigation Steps

change

if (tickTracking.exitTimestamp < nextWeek) {
    // Exit was in this week, continue with next tick
    tickActiveEnd = tickTracking.exitTimestamp;
    tickTrackingIndex++;
    dt = tickActiveEnd - tickActiveStart;
 }

to

if (tickTracking.exitTimestamp < nextWeek) {
    // Exit was in this week, continue with next tick
    tickActiveEnd = tickTracking.exitTimestamp;
    tickTrackingIndex++;
    dt = tickActiveEnd - time;
 }