Canto Liquidity Mining Protocol - SovaSlava's results

Lines of code

https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/callpaths/LiquidityMiningPath.sol#L78 https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/callpaths/LiquidityMiningPath.sol#L69

Vulnerability details

Impact

User could get reward more/less, than governance set, if reward value will be changed shortly before the end of the current period(current week).

Proof of Concept

Governance could change reward value of current perion. Sponsor said, that such situation is possible. Example 1 (User got reward more, than allowed): Block.timestamp = 1696584175 [Block.timestamp: 1696584175] - Governance call setAmbRewards(0x123, 1696464000, 1697068800, 100e18). [Block.timestamp: 1697068700] - Governance decide downward weeklyReward and call setAmbRewards with new value. setAmbRewards(0x123, 1696464000, 1697068800, 90e18) [Block.timestamp: 1697068801] - Next week is come. But tx from Governance still pending. User call LiquidityMiningPath.userCmd with args: code=102(claimAmbientRewards), weeksToClaim[] = [1696464000, 1697068800] and get reward with rewardAmount value, which was set at first Governance's tx - 100e18. [Block.timestamp: 1697068810] - Governance's tx mined. Now, weeklyReward value set as 90e18, but User already received a reward with a large coefficient. Because, Governance's tx was in mempool too much time and next week is come.

Example 2 (User got reward less, than allowed (0 -> 100): The bug is due to the fact that in the function claimAmbientRewards/claimConcentratedRewards, even if the rewardsToSend is 0, its store in mapping ambLiquidityRewardsClaimed_/concLiquidityRewardsClaimed_ that reward for week is received.

 if (overallTimeWeightedLiquidity > 0) {
                uint256 rewardsForWeek = (timeWeightedWeeklyPositionAmbLiquidity_[
                    poolIdx
                ][posKey][week] * ambRewardPerWeek_[poolIdx][week]) /
                    overallTimeWeightedLiquidity;
                rewardsToSend += rewardsForWeek;
            }
      
            ambLiquidityRewardsClaimed_[poolIdx][posKey][week] = true;  // <---------

The decision on the amount of reward was made only at the end of current period. Remeber, that default value of variable is 0. Block.timestamp = 1696584175 [Block.timestamp: 1696584175] - Governance call setAmbRewards(0x123, 1696464000, 1697068800, 100e18). [Block.timestamp: 1697068700] - Governance make decision about weeklyReward amount and call setAmbRewards. setAmbRewards(0x123, 1696464000, 1697068800, 100e18) [Block.timestamp: 1697068801] - Next week has come. But tx from Governance is still pending. User call LiquidityMiningPath.userCmd with args: code=102(claimAmbientRewards), weeksToClaim[] = [1696464000, 1697068800] and didnt get reward, because ambRewardPerWeek_[poolIdx][week] = 0. But, function store information in mapping, that user has received reward.

   if (overallTimeWeightedLiquidity > 0) {
                uint256 rewardsForWeek = (timeWeightedWeeklyPositionAmbLiquidity_[
                    poolIdx
                ][posKey][week] * ambRewardPerWeek_[poolIdx][week]) /
                    overallTimeWeightedLiquidity;
                rewardsToSend += rewardsForWeek; // <-- 0 * 200e18(for example) = 0
            }
      
            ambLiquidityRewardsClaimed_[poolIdx][posKey][week] = true; // <------

[Block.timestamp: 1697068810] - Governance's tx mined. Now, weeklyReward value set as 100e18, but if User try to recall own tx with the same parameters, it will be revert.

require(!ambLiquidityRewardsClaimed_[poolIdx][posKey][week],"Already claimed");

Tools Used

Manual review

Recommended Mitigation Steps

Prohibit changes of rewards for past periods

Assessed type

Invalid Validation