Canto Liquidity Mining Protocol - 3docSec's results

Lines of code

https://github.com/code-423n4/2023-10-canto/blob/37a1d64cf3a10bf37cbc287a22e8991f04298fa0/canto_ambient/contracts/mixins/TradeMatcher.sol#L442 https://github.com/code-423n4/2023-10-canto/blob/37a1d64cf3a10bf37cbc287a22e8991f04298fa0/canto_ambient/contracts/mixins/LiquidityMining.sol#L34 https://github.com/code-423n4/2023-10-canto/blob/37a1d64cf3a10bf37cbc287a22e8991f04298fa0/canto_ambient/contracts/mixins/LiquidityMining.sol#L94

Vulnerability details

Whenever a swap operations moves the nominal exchange price of the pool enough to cross a tick, LiquidityMining.sol keeps track of the event by storing a new TickTracking struct in its stored arrays:

        // TradeMatcher.sol:441 "sweepSwapLiq" function
        if (preTradeTick != postTradeTick) {
            crossTicks(pool.hash_, preTradeTick, postTradeTick);
        }

        // ----

        // LiquidityMining.sol:24
        function crossTicks(
            bytes32 poolIdx,
            int24 exitTick,
            int24 entryTick
        ) internal {
            uint256 numElementsExit = tickTracking_[poolIdx][exitTick].length;
            tickTracking_[poolIdx][exitTick][numElementsExit - 1]
                .exitTimestamp = uint32(block.timestamp);
            StorageLayout.TickTracking memory tickTrackingData = StorageLayout
                .TickTracking(uint32(block.timestamp), 0);
            tickTracking_[poolIdx][entryTick].push(tickTrackingData);
        }

These arrays of TickTracking structs are then looped over in the accrueConcentratedPositionTimeWeightedLiquidity, which is called by any liquidity event of a user's position (deposit or withdraw liquidity; claim rewards).

It is possible that a large number of swap operations create long lists of TickTracking objects stored, and it is also possible that enough of them are stored for a accrueConcentratedPositionTimeWeightedLiquidity call to exceed the maximum gas available in a single Canto block, making operations involving this call (including the MINT_RANGE and more importantly the BURN_RANGE user command that lets users withdraw their funds from the pool) inevitably fail.

To elaborate on the likelihood of this condition to happen, a single loop over a TickTracking object costs about 650 in gas and a block on the CANTO network is limited to 30M gas, so about 45k swaps are sufficient to freeze assets this way.

These swaps can happen during normal operations, because they don't need to happen in the same block or be triggered by the same actor to stack up, so an excessive volume can happen eventually without anybody having to intentionally orchestrate a costly attack.

Impact

The worst case impact is that users are unable to withdraw their liquidity from a heavily-traded position, because the withdraw operation would exceed the maximum gas allowed on a CANTO block.

Proof of Concept

The following test case shows how after 45k swaps, the gas of a deposit exceeds 30M gas. Deposit is shown for simplicity but withdrawal would fail the same way because it's the shared accrual logic that is at fault. The full runnable code in Foundry, including setup and helper methods is available here.

    function testImpossibleMint() public {
        address user1 = address(uint160(uint256(keccak256("user1"))));
        provideLiquidity(user1, 100e18);

        uint256 currentTs = block.timestamp;
        vm.pauseGasMetering();

        for(uint i = 0; i < 23_000; i++) {
            currentTs = currentTs + 1 minutes;
            vm.warp(currentTs);
            swapUsdcForCnote();

            currentTs = currentTs + 1 minutes;
            vm.warp(currentTs);
            swapCnoteForUsdc();
        }

        vm.resumeGasMetering();

        // 
        /*
          The below fails for out of gas, exceeding the 30M gas it was given:
        |   ├─ [29531439] CrocSwapDex::userCmd(2, 0x...)
        │   ├─ [29526362] WarmPath::userCmd(0x...) [delegatecall]
        │   │   └─ ← "EvmError: OutOfGas"
        */
        provideLiquidity(user1, 1);
    }

Tools Used

Code review, Foundry

Recommended Mitigation Steps

Right now, tick snapshots are taken at a potentially infinite granularity, because a group of TickTracking objects can very well start and end at the same block and have no use because rewards are calculated on the gap between their start and end.

The most immediate way to mitigate this issue is to aggregate the data on time windows less granular than this, but still more granular than weeks (i.e. with hourly granularity).

Assessed type

Loop

[L-01] LiquidityMiningPath.protocolCmd should be payable The function LiquidityMiningPath.protocolCmd is called by CrocSwapDex.protocolCmd which is payable. Consider making it payable too, especially because this way it would be possible to enable and fund rewards with one protocolCmd call.

[N-01] Consider removing the hardhat console import from production code LevelBook.sol imports the hardhat console. Consider removing this import before production deployment.