Lines of code

Vulnerability details

Impact

The _calculateNewRewards function in the smart contract performs a division operation with totalInterestEarnedInPeriod, which could be zero, but there is a problem that can result in a division-by-zero error, causing the smart contract to behave unexpectedly or crash.
The problem is with the division operation if totalInterestEarnedInPeriod is zero. This is because dividing any number by zero is undefined, and will result in a runtime error in Solidity.
RewardsManager.sol#L535-L539

        newRewards_ = totalInterestEarnedInPeriod == 0 ? 0 : Maths.wmul(
            REWARD_FACTOR,
            Maths.wdiv(
                Maths.wmul(interestEarned_, totalBurnedInPeriod),
                totalInterestEarnedInPeriod
            )
        );

In this code, if totalInterestEarnedInPeriod is zero, then the conditional operator totalInterestEarnedInPeriod == 0 ? 0 : Maths.wmul(…) will return 0 as the value of newRewards_. However, the division operation inside the Maths.wdiv(…) function will still be executed, because the code does not check whether totalInterestEarnedInPeriod is zero before performing the division.

If totalInterestEarnedInPeriod is zero, then the result of the division operation will be undefined, and will cause a runtime error. This could potentially crash the contract or cause unexpected behavior.

Proof of Concept

RewardsManager.sol

function _calculateNewRewards(
        address ajnaPool_,
        uint256 interestEarned_,
        uint256 nextEpoch_,
        uint256 epoch_,
        uint256 rewardsClaimedInEpoch_
    ) internal view returns (uint256 newRewards_) {
        (
            ,
            // total interest accumulated by the pool over the claim period
            uint256 totalBurnedInPeriod,
            // total tokens burned over the claim period
            uint256 totalInterestEarnedInPeriod
        ) = _getPoolAccumulators(ajnaPool_, nextEpoch_, epoch_);

        // calculate rewards earned
        newRewards_ = totalInterestEarnedInPeriod == 0 ? 0 : Maths.wmul(
            REWARD_FACTOR,
            Maths.wdiv(
                Maths.wmul(interestEarned_, totalBurnedInPeriod),
                totalInterestEarnedInPeriod
            )
        );

        uint256 rewardsCapped = Maths.wmul(REWARD_CAP, totalBurnedInPeriod);

        // Check rewards claimed - check that less than 80% of the tokens for a given burn event have been claimed.
        if (rewardsClaimedInEpoch_ + newRewards_ > rewardsCapped) {

            // set claim reward to difference between cap and reward
            newRewards_ = rewardsCapped - rewardsClaimedInEpoch_;
        }
    }

The totalInterestEarnedInPeriod variable is used as a divisor in the division operation performed by the Maths.wdiv(…) function. If totalInterestEarnedInPeriod is zero, this division operation will result in a division-by-zero error.

For example
Let’s say Alice and Bob are users of the RewardsManager.sol contract that uses the vulnerable function _calculateNewRewards. Alice has deposited 10 tokens into the contract, and Bob has deposited 5 tokens. The contract has a total of 15 tokens in it.
Let’s say the contract has earned 1 token in interest, and the reward factor is set to 0.5. According to the formula used in _calculateNewRewards, Alice and Bob should each receive a reward of 0.25 tokens:

totalBurnedInPeriod = 15 tokens
totalInterestEarnedInPeriod = 1 token
interestEarned_ = 0.5 tokens

newRewards_ = Maths.wmul(
    REWARD_FACTOR,
    Maths.wdiv(
        Maths.wmul(interestEarned_, totalBurnedInPeriod),
        totalInterestEarnedInPeriod
    )
) = Maths.wmul(0.5, Maths.wdiv(Maths.wmul(0.5, 15), 1)) = 1.875 tokens

rewardsCapped = Maths.wmul(REWARD_CAP, totalBurnedInPeriod) = Maths.wmul(2, 15) = 30 tokens

// Check rewards claimed
if (0 + 1.875 > 30) {
    newRewards_ = 30 - 0 = 30 tokens
}

However, an attacker can call the function with totalInterestEarnedInPeriod set to zero, which will result in a division-by-zero error. Let’s say the attacker calls the function with totalInterestEarnedInPeriod set to zero, and with the same inputs as before:

totalBurnedInPeriod = 15 tokens
totalInterestEarnedInPeriod = 0 tokens
interestEarned_ = 0.5 tokens

newRewards_ = Maths.wmul(
    REWARD_FACTOR,
    Maths.wdiv(
        Maths.wmul(interestEarned_, totalBurnedInPeriod),
        totalInterestEarnedInPeriod
    )
) = Maths.wmul(0.5, Maths.wdiv(Maths.wmul(0.5, 15), 0)) = Division by zero!

rewardsCapped = Maths.wmul(REWARD_CAP, totalBurnedInPeriod) = Maths.wmul(2, 15) = 30 tokens

// Check rewards claimed
if (0 + newRewards_ > 30) {
    newRewards_ = 30 - 0 = 30 tokens
}

As a result of the division-by-zero error, the contract will behave unexpectedly or may crash. The attacker can then manipulate the reward calculation and potentially steal rewards from the contract’s users.

Tools Used

vscode

Recommended Mitigation Steps

it is recommended to add a check to ensure that totalInterestEarnedInPeriod is non-zero before performing the division operation. This can be done by adding an if statement to check if totalInterestEarnedInPeriod is zero, and if so, returning zero or reverting the transaction with an error message.

Assessed type

Under/Overflow

---

The text was updated successfully, but these errors were encountered:

All reactions