code-423n4 / 2023-10-canto-findings

0 stars 1 forks source link

Potential Arithmetic Overflow in Rewards Calculation #208

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

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

Vulnerability details

Impact

The code in the provided smart contract may be susceptible to arithmetic overflow vulnerabilities, particularly in the calculation of rewards (rewardsToSend) and certain liquidity-related variables. In the event of an overflow, it could lead to incorrect rewards calculations, financial losses, or unexpected behavior within the contract.

Proof of Concept

Overflow in rewardsToSend: The code calculates rewardsForWeek by multiplying timeWeightedWeeklyPositionAmbLiquidity[poolIdx][posKey][week] by ambRewardPerWeek[poolIdx][week] and then divides it by overallTimeWeightedLiquidity. If any of these values are very large, it could lead to an overflow in the rewardsForWeek variable, potentially resulting in incorrect rewards calculation or unexpected behavior.

function claimAmbientRewards(
    address owner,
    bytes32 poolIdx,
    uint32[] memory weeksToClaim
) internal {
...
        uint256 overallTimeWeightedLiquidity = timeWeightedWeeklyGlobalAmbLiquidity_[
                poolIdx
            ][week];
        if (overallTimeWeightedLiquidity > 0) {
            uint256 rewardsForWeek = (timeWeightedWeeklyPositionAmbLiquidity_[
                poolIdx
            ][posKey][week] * ambRewardPerWeek_[poolIdx][week]) /
                overallTimeWeightedLiquidity;
            rewardsToSend += rewardsForWeek;
        }
...
}

Overflow in timeWeightedWeeklyPositionAmbLiquidity and time : Similarly, if the timeWeightedWeeklyPositionAmbLiquidity[poolIdx][posKey][currWeek] value is very large, it may lead to an overflow, affecting the correctness of the calculations.

function accrueAmbientPositionTimeWeightedLiquidity(
    address payable owner,
    bytes32 poolIdx
) internal {
    ..
            timeWeightedWeeklyPositionAmbLiquidity_[poolIdx][posKey][
                currWeek
            ] += dt * liquidity;
            time += dt;
        }
    }

Tools Used

Manual review

Recommended Mitigation Steps

To mitigate the potential arithmetic overflow issue in the rewards calculation you can use the OpenZeppelin SafeMath library

Assessed type

Under/Overflow

c4-pre-sort commented 1 year ago

141345 marked the issue as low quality report

141345 commented 1 year ago

seems invalid

timeWeightedWeeklyPositionAmbLiquidity_ and dt * liquidity are not likely to be that huge amount

c4-judge commented 1 year ago

dmvt marked the issue as unsatisfactory: Invalid