Canto Liquidity Mining Protocol - ni8mare's results

Lines of code

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

Vulnerability details

Impact

In the function accrueConcentratedPositionTimeWeightedLiquidity, inside the while block, dt is initialised as:

    uint32 dt = uint32(
       nextWeek < block.timestamp
       ? nextWeek - time
       : block.timestamp - time
   );

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

If tickTracking.exitTimestamp != 0 then the following else block is executed on line 117:

else {
   // Tick is no longer active
   if (tickTracking.exitTimestamp < nextWeek) {
      // Exit was in this week, continue with next tick
      tickActiveEnd = tickTracking.exitTimestamp;
      tickTrackingIndex++;
      dt = tickActiveEnd - tickActiveStart;
   } else {
     // Exit was in next week, we need to consider the current tick there (i.e. not increase the index)
     tickActiveEnd = nextWeek;
    }
 }

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

dt is now updated to tickActiveEnd - tickActiveStart; when tickTracking.exitTimestamp < nextWeek as seen in the if block of the outer else block above.

But, when tickTracking.exitTimestamp > nextWeek, in the inner else block, the value of dt is not updated:

 else {
     // Exit was in next week, we need to consider the current tick there (i.e. not increase the index)
     tickActiveEnd = nextWeek;
     //@audit - No update on dt value
    }

Inside this else block as well, dt must equal tickActiveEnd - tickActiveStart;, where tickActiveEnd = nextWeek;. Without this update if tickTracking.exitTimestamp > nextWeek, then dt = nextWeek - time or dt = block.timestamp - time as it was declared initially. For example, let's say that it was the former, that is, dt = nextWeek - time (according to the initial declaration). Assume that tickActiveStart = tickTracking.enterTimestamp (this is possible when tickTracking.enterTimestamp > time). Had the else block above (check @audit tag), updated dt, then dt = tickActiveEnd - tickActiveStart; => dt = nextWeek - tickTracking.enterTimestamp

So, on comparing again, initially -> dt = nextWeek - time and had there been an update on dt at the audit tag, dt would now be -> dt = nextWeek - tickTracking.enterTimestamp. Note that here, tickTracking.enterTimestamp > time (check the above para). So, nextWeek - tickTracking.enterTimestamp < nextWeek - time. That means dt would be a smaller value had it been equal to nextWeek - tickTracking.enterTimestamp. But, since its not updated, dt equals nextWeek - time, which is a bigger value.

dt is used to increase the value of time (used as an iterator in while loop). Since dt is a bigger value than required, the number of iterations of the while loop would be less than what it should be. This means that fewer iterations would be used to update timeWeightedWeeklyPositionInRangeConcLiquidity_ on line 129

timeWeightedWeeklyPositionInRangeConcLiquidity_[poolIdx][posKey][currWeek][i] +=
                            (tickActiveEnd - tickActiveStart) * liquidity;

timeWeightedWeeklyPositionInRangeConcLiquidity_ is used to calculate the rewardsToSend value in claimConcentratedRewards function. If timeWeightedWeeklyPositionInRangeConcLiquidity_ is incorrect, then the rewards to be sent to the user will be calculated incorrectly.

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

uint256 overallInRangeLiquidity = timeWeightedWeeklyGlobalConcLiquidity_[poolIdx][week];
   if (overallInRangeLiquidity > 0) {
      uint256 inRangeLiquidityOfPosition;
      for (int24 j = lowerTick + 10; j <= upperTick - 10; ++j) {
         inRangeLiquidityOfPosition += timeWeightedWeeklyPositionInRangeConcLiquidity_[poolIdx][posKey][week][j];
       }
       // Percentage of this weeks overall in range liquidity that was provided by the user times the overall weekly rewards
       rewardsToSend += inRangeLiquidityOfPosition * concRewardPerWeek_[poolIdx][week] / overallInRangeLiquidity; //@audit - rewards to send will be less

Also, due to fewer number of iterations, tickTrackingIndexAccruedUpTo_ might not be updated correctly.

if (tickTrackingIndex != origIndex) {
  tickTrackingIndexAccruedUpTo_[poolIdx][posKey][i] = tickTrackingIndex;
}

Proof of Concept

    /// @notice Accrues the in-range time-weighted concentrated liquidity for a position by going over the tick entry / exit history
    /// @dev Needs to be called whenever a position is modified
    function accrueConcentratedPositionTimeWeightedLiquidity(
        address payable owner,
        bytes32 poolIdx,
        int24 lowerTick,
        int24 upperTick
    ) internal {
        RangePosition72 storage pos = lookupPosition(
            owner,
            poolIdx,
            lowerTick,
            upperTick
        );
        bytes32 posKey = encodePosKey(owner, poolIdx, lowerTick, upperTick);
        uint32 lastAccrued = timeWeightedWeeklyPositionConcLiquidityLastSet_[
            poolIdx
        ][posKey];
        // Only set time on first call
        if (lastAccrued != 0) {
            uint256 liquidity = pos.liquidity_;
            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;
                // Loop through all in-range time spans for the tick or up to the current time (if it is still in range)
                while (time < block.timestamp && tickTrackingIndex < numTickTracking) {
                    TickTracking memory tickTracking = tickTracking_[poolIdx][i][tickTrackingIndex];
                    uint32 currWeek = uint32((time / WEEK) * WEEK);
                    uint32 nextWeek = uint32(((time + WEEK) / WEEK) * WEEK);
                    uint32 dt = uint32(
                        nextWeek < block.timestamp
                            ? nextWeek - time
                            : block.timestamp - time
                    );
                    uint32 tickActiveStart; // Timestamp to use for the liquidity addition
                    uint32 tickActiveEnd;
                    if (tickTracking.enterTimestamp < nextWeek) {
                        // Tick was active before next week, need to add the liquidity
                        if (tickTracking.enterTimestamp < time) {
                            // Tick was already active when last claim happened, only accrue from last claim timestamp
                            tickActiveStart = time;
                        } else {
                            // Tick has become active this week
                            tickActiveStart = tickTracking.enterTimestamp;
                        }
                        if (tickTracking.exitTimestamp == 0) {
                            // Tick still active, do not increase index because we need to continue from here
                            tickActiveEnd = uint32(nextWeek < block.timestamp ? nextWeek : block.timestamp);
                        } else {
                            // Tick is no longer active
                            if (tickTracking.exitTimestamp < nextWeek) {
                                // Exit was in this week, continue with next tick
                                tickActiveEnd = tickTracking.exitTimestamp;
                                tickTrackingIndex++;
                                dt = tickActiveEnd - tickActiveStart;
                            } else {
                                // Exit was in next week, we need to consider the current tick there (i.e. not increase the index)
                                tickActiveEnd = nextWeek;
                            }
                        }
                        timeWeightedWeeklyPositionInRangeConcLiquidity_[poolIdx][posKey][currWeek][i] +=
                            (tickActiveEnd - tickActiveStart) * liquidity;
                    }
                    time += dt;
                }
                if (tickTrackingIndex != origIndex) {
                    tickTrackingIndexAccruedUpTo_[poolIdx][posKey][i] = tickTrackingIndex;
                }
            }
        } else {
            for (int24 i = lowerTick + 10; i <= upperTick - 10; ++i) {
                uint32 numTickTracking = uint32(tickTracking_[poolIdx][i].length);
                if (numTickTracking > 0) {
                    if (tickTracking_[poolIdx][i][numTickTracking - 1].exitTimestamp == 0) {
                        // Tick currently active
                        tickTrackingIndexAccruedUpTo_[poolIdx][posKey][i] = numTickTracking - 1;
                    } else {
                        tickTrackingIndexAccruedUpTo_[poolIdx][posKey][i] = numTickTracking;
                    }
                }
            }
        }
        timeWeightedWeeklyPositionConcLiquidityLastSet_[poolIdx][
            posKey
        ] = uint32(block.timestamp);
    }

Tools Used

Manual review

Recommended Mitigation Steps

add the line - dt = tickActiveEnd - tickActiveStart in the place of the audit tag as shown above.

Assessed type

Other