The lender won't be able to claim rewards in some cases and most of RewardsManager's methods (e.g. staking, unstaking ..etc) will revert.

[code423n4]

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-core/src/RewardsManager.sol#L719-L726

Vulnerability details

Impact

In RewardsManager, when claiming the rewards, _updateBucketExchangeRates method is called. This method does the following:

1. If curBurnEpoch == 0 then just update the rates without calculating any reward.
2. Otherwise, retrieve accumulator values (e.g. totalBurned)

	// retrieve accumulator values used to calculate rewards accrued
	(
		uint256 curBurnTime,
		uint256 totalBurned,
		uint256 totalInterestEarned
	) = _getPoolAccumulators(pool_, curBurnEpoch, curBurnEpoch - 1);

3. Loop through the bucket index (positions recorded for the lender)
   • For each index, update bucket exchange rate and calculate the rewards (done via _updateBucketExchangeRateAndCalculateRewards)
   • Add the rewards to updatedRewards_ to track the total rewards.

	for (uint256 i = 0; i < indexes_.length; ) {

		// calculate rewards earned for updating bucket exchange rate
		updatedRewards_ += _updateBucketExchangeRateAndCalculateRewards(
			pool_,
			indexes_[i],
			curBurnEpoch,
			totalBurned,
			totalInterestEarned
		);

		// iterations are bounded by array length (which is itself bounded), preventing overflow / underflow
		unchecked { ++i; }
	}

4. Calculate the rewards cap based on totalBurned

	uint256 rewardsCap            = Maths.wmul(UPDATE_CAP, totalBurned);

5. Make sure old updateRewardsClaimed in epoch + updatedRewards_ doesn't exceed the rewards cap.

	// update total tokens claimed for updating bucket exchange rates tracker
	if (rewardsClaimedInEpoch + updatedRewards_ >= rewardsCap) {
		// if update reward is greater than cap, set to remaining difference
		updatedRewards_ = rewardsCap - rewardsClaimedInEpoch;
	}

6. Add updatedRewards_ to updateRewardsClaimed in epoch

	// accumulate the full amount of additional rewards
	updateRewardsClaimed[curBurnEpoch] += updatedRewards_;

Since rewardsClaimedInEpoch is always supposed to be not be greater than rewardsCap we have no revert issue (note: the method reverts if rewardsCap is lower than rewardsClaimedInEpoch). However, to guarantee this, totalBurned should never decrease. In other words, if totalBurned went from a high to low value at any point, then rewardsCap will follow. If we check how totalBurned is calculated, we notice that there is a possibilty that this happens.

	uint256 totalBurned   = totalBurnedLatest   != 0 ? totalBurnedLatest   - totalBurnedAtBlock   : totalBurnedAtBlock;

totalBurnedLatest => total burned of current epoch
totalBurnedAtBlock => total burned of previous epoch

If totalBurnedLatest equals zero, it takes the difference between totalBurnedLatest and totalBurnedAtBlock. otherwise, it takes totalBurnedAtBlock. because of this logic, it is possible that we have the following:

• Assume totalBurnedLatest = 1500 and totalBurnedAtBlock = 1000
  • totalBurned = 500
• totalBurnedAtBlock increased with 100
  • totalBurned = 400

Now. since totalBurned went lower, rewardsCap will go lower as well.
The issue is that, in case the above happens, updateRewardsClaimed in epoch will be greater than rewardsCap and this condition will be true regardless of how much updatedRewards_ the user should receive

rewardsClaimedInEpoch + updatedRewards_ >= rewardsCap

This results in a revert in the following line since rewardsCap is lower than rewardsClaimedInEpoch:

	// if update reward is greater than cap, set to remaining difference
	updatedRewards_ = rewardsCap - rewardsClaimedInEpoch;

Therefore, the user won't be able to claim rewards in such situations. And since claim rewards is also done upon staking, unstaking and moveStakedLiquidity, all of those will revert due to this issue.

Proof of Concept

Please check above for explanation.
https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-core/src/RewardsManager.sol#L719-L726

Tools Used

Manual analysis

Recommended Mitigation Steps

Check if rewardsCap is lower than rewardsClaimedInEpoch, set updatedRewards_ to zero.

Assessed type

DoS

[c4-sponsor]

MikeHathaway marked the issue as sponsor confirmed

[c4-judge]

Picodes marked the issue as duplicate of #440

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes changed the severity to 3 (High Risk)