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
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.
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:
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:
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.
Lines of code
https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-core/src/RewardsManager.sol#L535-L541
Vulnerability details
Impact
The
_calculateNewRewards
function in the smart contract performs a division operation withtotalInterestEarnedInPeriod
, 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 iftotalInterestEarnedInPeriod
is zero. This is because dividing any number by zero is undefined, and will result in a runtime error in Solidity. RewardsManager.sol#L535-L539In this code, if
totalInterestEarnedInPeriod
is zero, then the conditional operatortotalInterestEarnedInPeriod == 0 ? 0 : Maths.wmul(...)
will return0
as the value ofnewRewards_
. However, the division operation inside theMaths.wdiv(...)
function will still be executed, because the code does not check whethertotalInterestEarnedInPeriod
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
The
totalInterestEarnedInPeriod
variable is used as a divisor in the division operation performed by theMaths.wdiv(...)
function. IftotalInterestEarnedInPeriod
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 deposited10 tokens
into the contract, and Bob has deposited5 tokens
. The contract has a total of15 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: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 withtotalInterestEarnedInPeriod
set to zero, and with the same inputs as before: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 iftotalInterestEarnedInPeriod
is zero, and if so, returning zero or reverting the transaction with an error message.Assessed type
Under/Overflow