code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

File: RewardsManager.sol
177             // update to bucket state
178             BucketState storage toBucket = stakeInfo.snapshot[toIndex];
179             toBucket.lpsAtStakeTime  = uint128(positionManager.getLP(tokenId_, toIndex));
180 >>          toBucket.rateAtStakeTime = uint128(IPool(ajnaPool).bucketExchangeRate(toIndex));
181             delete stakeInfo.snapshot[fromIndex];

The moveStakedLiquidity function will update toBucket.rateAtStakeTime to the current ExchangeRate every time.

File: RewardsManager.sol
445             if (epoch_ != stakingEpoch_) {
446
447                 // if staked in a previous epoch then use the initial exchange rate of epoch
448                 bucketRate = bucketExchangeRates[ajnaPool_][bucketIndex][epoch_];
449             } else {
450
451                 // if staked during the epoch then use the bucket rate at the time of staking
452                 bucketRate = bucketSnapshot.rateAtStakeTime;
453             }

If an epoch is the first epoch for a position, the bucketRate is bucketSnapshot.rateAtStakeTime, otherwise the bucketRate is uniformly bucketExchangeRates[ajnaPool_][bucketIndex][epoch_]. So in a certain epoch, the reward ratio of positions is the same in most cases. But for positions staked in this epoch, the reward rate is different and depends on rateAtStakeTime.

An attacker can use the moveStakedLiquidity function to update rateAtStakeTime to obtain the maximum reward ratio. This makes his reward potentially much higher than other positions in the epoch, and the excess is stolen from other user rewards.

Proof of Concept

epoch N starts
Alias stake a position A worth 10,000 LP, position A's rateAtStakeTime is Rate0
Alias has been checking the bucketExchangeRate, and once it finds that the bucketExchangeRate is less than Rate0, it will call moveStakedLiquidity to transfer 1 LP to position A, and the rateAtStakeTime of position A will be updated to Rate1
Repeat the step 3 until the end of epoch N
Alias unstake after epoch N, Alias get a larger reward ratio than other users

Tools Used

Manual review

Recommended Mitigation Steps

It is recommended to update the bucketRate according to the ratio of moved LP to original LP.

Assessed type

Other

Picodes commented 1 year ago

Related to #345 and #346 by the same warden

c4-judge commented 1 year ago

Picodes changed the severity to 2 (Med Risk)

c4-sponsor commented 1 year ago

MikeHathaway marked the issue as sponsor disputed

MikeHathaway commented 1 year ago

These aren't real attacks. If you move your tokens, you should get re stamped, and only earn interest from that point forward. This is by design, and is actually to avoid gaming the system, by joining buckets post hoc that earned interest