Canto Liquidity Mining Protocol - 0xWaitress's results

Lines of code

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

Vulnerability details

Impact

accrueConcentratedGlobalTimeWeightedLiquidity and accrueConcentratedPositionTimeWeightedLiquidity has unbounded while loop; potentially out of gas error if not updated for a long time

liquidity is calculated before it 's changed to count the total time weighted liquidity provision eligible for incentive. They are accrued in both accrueConcentratedGlobalTimeWeightedLiquidity and accrueConcentratedPositionTimeWeightedLiquidity on a per-week basis.

However the liquidity accrual can only be done all the way from the last accrual to the latest block timestamp, in an unbounded while loop, this is problematic since if the pool is not touched for long time then the gas for the update may become bigger than the gas limit / limit for a single tx such that the pool becomes no longer updateable.

    function accrueAmbientPositionTimeWeightedLiquidity(
        address payable owner,
        bytes32 poolIdx
    ) internal {
        bytes32 posKey = encodePosKey(owner, poolIdx);
        uint32 lastAccrued = timeWeightedWeeklyPositionAmbLiquidityLastSet_[
            poolIdx
        ][posKey];
        // Only init time on first call
        if (lastAccrued != 0) {
            AmbientPosition storage pos = lookupPosition(owner, poolIdx);
            uint256 liquidity = pos.seeds_;
            uint32 time = lastAccrued;
            while (time < block.timestamp) {
                uint32 currWeek = uint32((time / WEEK) * WEEK);
                uint32 nextWeek = uint32(((time + WEEK) / WEEK) * WEEK);
                uint32 dt = uint32(
                    nextWeek < block.timestamp
                        ? nextWeek - time
                        : block.timestamp - time
                );
                timeWeightedWeeklyPositionAmbLiquidity_[poolIdx][posKey][
                    currWeek
                ] += dt * liquidity;
                time += dt;
            }
        }
        timeWeightedWeeklyPositionAmbLiquidityLastSet_[poolIdx][
            posKey
        ] = uint32(block.timestamp);
    }

Proof of Concept

Tools Used

Recommended Mitigation Steps

Consider updating the liquidity in a checkpoint manner (in 1 go), and liquidity can be tracked through iterating the needed pointer using a binary search.

Assessed type

Loop

[L-1] unnecessary exposure of setAmbRewards and setConcRewards in LiquidityMiningPath as a public function without access controller

While LiquidityMiningPath is a contract that the CrocSwapDex delegates userCmd or protocolCmd only, and all execution is meant to be done regarding storage on CrocSwapDex, the calling of setAmbRewards and setConcRewards on LiquidityMiningPath does not have access control as a public function that everyone can access. This let other to arbitrarily make change to the storage of relevant data like ambRewardPerWeek_ and concRewardPerWeek_ on the LiquidityMiningPath contract. While this does no harm the operation, this is unnecessary and create additional vulnerability if the logic is upgraded in future to include initialiser / or function that allow self-destruct or changes of the module with reference to its own data.