The calculateNewRewards function should also check whether the rewardsCapped variable is greater than the rewardsClaimedInEpoch_ variable to ensure that rewards are not claimed that exceed the reward cap, because an attacker could exploit this vulnerability by claiming more rewards than allowed by the reward cap, resulting in a loss of funds for the pool and negatively affecting the overall functionality of the smart contract.
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_;
}
}
Because the calculateNewRewards function checks whether the newRewards variable is greater than the rewardsCapped variable, but it does not check whether the rewardsCapped variable is greater than the rewardsClaimedInEpoch_ variable. As a result, an attacker could potentially claim more rewards than allowed by the reward cap, resulting in a loss of funds for the pool.
Tools Used
vscode
Recommended Mitigation Steps
Add an additional check before the if statement that checks whether rewardsCapped is greater than rewardsClaimedInEpoch_. If it is not, then newRewards_ should be set to 0, indicating that there are no rewards left to claim in the current epoch.
Lines of code
https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-core/src/RewardsManager.sol#L519-L551
Vulnerability details
Impact
The
calculateNewRewards
function should also check whether therewardsCapped
variable is greater than therewardsClaimedInEpoch_
variable to ensure that rewards are not claimed that exceed the reward cap, because an attacker could exploit this vulnerability by claiming more rewards than allowed by the reward cap, resulting in a loss of funds for the pool and negatively affecting the overall functionality of the smart contract.Proof of Concept
RewardsManager.sol#L519-L551
Because the
calculateNewRewards
function checks whether thenewRewards
variable is greater than therewardsCapped
variable, but it does not check whether therewardsCapped
variable is greater than therewardsClaimedInEpoch_
variable. As a result, an attacker could potentially claim more rewards than allowed by the reward cap, resulting in a loss of funds for the pool.Tools Used
vscode
Recommended Mitigation Steps
Add an additional check before the
if
statement that checks whetherrewardsCapped
is greater thanrewardsClaimedInEpoch_
. If it is not, thennewRewards_
should be set to 0, indicating that there are no rewards left to claim in the current epoch.Assessed type
Access Control