Potential Reward Draining Vulnerability in claimAmbientRewards Function

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-canto/blob/main/canto_ambient/contracts/callpaths/LiquidityMiningPath.sol#L62 https://github.com/code-423n4/2023-10-canto/blob/main/canto_ambient/contracts/callpaths/LiquidityMiningPath.sol#L58 https://github.com/code-423n4/2023-10-canto/blob/main/canto_ambient/contracts/mixins/LiquidityMining.sol#L256 https://github.com/code-423n4/2023-10-canto/blob/main/canto_ambient/contracts/mixins/LiquidityMining.sol#L156

Vulnerability details

Impact

The claimAmbientRewards function in the provided code has a potential vulnerability that could allow a malicious caller to drain the contract's balance by claiming rewards for future weeks. This could lead to a significant loss of funds and disrupt the intended behavior of the contract.

Proof of Concept

The claimAmbientRewards function is a public function that calls the internal claimAmbientRewards function. It allows any external caller to invoke the claim rewards process.

In the claimAmbientRewards function, the caller can specify an array of weeksToClaim. For each week specified in this array, the code calculates the rewards to send to the caller based on the ambRewardPerWeek and timeWeightedWeeklyPositionAmbLiquidity values.

The potential vulnerability lies in the fact that the caller can specify any week in the weeksToClaim array, and the rewards for that week will be calculated and sent to the caller. This means that the caller can potentially claim rewards for future weeks that have not yet occurred.

function claimAmbientRewards(bytes32 poolIdx, uint32[] memory weeksToClaim) public payable {
    claimAmbientRewards(payable(msg.sender), poolIdx, weeksToClaim);
}

function claimAmbientRewards(
    address owner,
    bytes32 poolIdx,
    uint32[] memory weeksToClaim
) internal {
    CurveMath.CurveState memory curve = curves_[poolIdx];
    accrueAmbientPositionTimeWeightedLiquidity(payable(owner), poolIdx);
    accrueAmbientGlobalTimeWeightedLiquidity(poolIdx, curve);
    bytes32 posKey = encodePosKey(owner, poolIdx);
    uint256 rewardsToSend;
    for (uint256 i; i < weeksToClaim.length; ++i) {
        uint32 week = weeksToClaim[i];
        require(week + WEEK < block.timestamp, "Week not over yet");
        require(
            !ambLiquidityRewardsClaimed_[poolIdx][posKey][week],
            "Already claimed"
        );
        uint256 overallTimeWeightedLiquidity = timeWeightedWeeklyGlobalAmbLiquidity_[
                poolIdx
            ][week];
        if (overallTimeWeightedLiquidity > 0) {
            uint256 rewardsForWeek = (timeWeightedWeeklyPositionAmbLiquidity_[
                poolIdx
            ][posKey][week] * ambRewardPerWeek_[poolIdx][week]) /
                overallTimeWeightedLiquidity;
            rewardsToSend += rewardsForWeek;
        }
        ambLiquidityRewardsClaimed_[poolIdx][posKey][week] = true;
    }
    if (rewardsToSend > 0) {
        (bool sent, ) = owner.call{value: rewardsToSend}("");
        require(sent, "Sending rewards failed");
    }
}

same for

function claimConcentratedRewards(bytes32 poolIdx, int24 lowerTick, int24 upperTick, uint32[] memory weeksToClaim)
    public
    payable
{
    claimConcentratedRewards(payable(msg.sender), poolIdx, lowerTick, upperTick, weeksToClaim);
}

function claimConcentratedRewards(
    address payable owner,
    bytes32 poolIdx,
    int24 lowerTick,
    int24 upperTick,
    uint32[] memory weeksToClaim
) internal {
    accrueConcentratedPositionTimeWeightedLiquidity(
        owner,
        poolIdx,
        lowerTick,
        upperTick
    );
    CurveMath.CurveState memory curve = curves_[poolIdx];
    // Need to do a global accrual in case the current tick was already in range for a long time without any modifications that triggered an accrual
    accrueConcentratedGlobalTimeWeightedLiquidity(poolIdx, curve);
    bytes32 posKey = encodePosKey(owner, poolIdx, lowerTick, upperTick);
    uint256 rewardsToSend;
    for (uint256 i; i < weeksToClaim.length; ++i) {
        uint32 week = weeksToClaim[i];
        require(week + WEEK < block.timestamp, "Week not over yet");
        require(
            !concLiquidityRewardsClaimed_[poolIdx][posKey][week],
            "Already claimed"
        );
        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;
        }
        concLiquidityRewardsClaimed_[poolIdx][posKey][week] = true;
    }
    if (rewardsToSend > 0) {
        (bool sent, ) = owner.call{value: rewardsToSend}("");
        require(sent, "Sending rewards failed");
    }
}

Tools Used

Manual review

Recommended Mitigation Steps

Time-Based Validation: Implement time-based validation checks in the claimAmbientRewards function to ensure that rewards can only be claimed for past weeks, not for future weeks. Use the current block timestamp to verify the claimed weeks.

Limit Rewards: Consider implementing a mechanism to limit the maximum amount of rewards that can be claimed in a single transaction. This can prevent excessive claims and limit potential damage.

Assessed type

Access Control

[c4-pre-sort]

141345 marked the issue as low quality report

[141345]

invalid

claim future rewards

        require(week + WEEK < block.timestamp, "Week not over yet");

[c4-judge]

dmvt marked the issue as unsatisfactory: Invalid