Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-core/src/RewardsManager.sol#L337-L349

Vulnerability details

Impact

In the calculateRewards function, there is an unchecked increment in the for loop, in the code it can allow an attacker to cause an integer overflow in the calculateRewards function by manipulating the loop variable, resulting in incorrect rewards calculation or even causing the contract to be stuck in an infinite loop.

Proof of Concept

The code block is present in the calculateRewards function: RewardsManager.sol#L337-L347

        for (uint256 epoch = lastClaimedEpoch; epoch < epochToClaim_; ) {

            rewards_ += _calculateNextEpochRewards(
                tokenId_,
                epoch,
                stakingEpoch,
                ajnaPool,
                positionIndexes
            );

            unchecked { ++epoch; }
        }
    }

There is an unchecked increment in the loop variable epoch. This can lead to unexpected behavior if the loop variable overflows. An attacker can potentially manipulate the loop variable to cause an integer overflow, resulting in incorrect rewards calculation or even causing the contract to be stuck in an infinite loop.

For example, if the epochToClaim_ variable is set to the maximum value of a uint256 variable, an attacker can set the lastClaimedEpoch variable to a value close to the maximum value and cause an integer overflow in the loop variable, resulting in incorrect rewards calculation or even causing the contract to be stuck in an infinite loop.

Tools Used

Manual code review.

Recommended Mitigation Steps

Consider adding a check to ensure that the loop variable does not overflow before incrementing it. One way to do this is by using a require statement to check if the incremented value is less than the maximum allowed value for the epoch variable.

for (uint256 epoch = lastClaimedEpoch; epoch < epochToClaim_; ) {
    rewards_ += _calculateNextEpochRewards(
        tokenId_,
        epoch,
        stakingEpoch,
        ajnaPool,
        positionIndexes
    );

    epoch = epoch + 1;
    require(epoch < type(uint256).max, "Epoch variable overflow");
}

This will ensure that the loop variable does not overflow, and if it does, the function will revert with an error message indicating that the overflow has occurred.

Assessed type

Under/Overflow

code-423n4 / 2023-05-ajna-findings

Unchecked increment in calculateRewards function of RewardsManager.sol. #432