Summary
A greifing user can frontrun other users' claiming transactions to make the rewards to be claimed less than expected at a relative low cost.
Vulnerability Details
In Distrubition.sol, users can stake stETH to the protocol, and after a while, can claim reward in MOR tokens. The reward calculation is based on userData.rate and poolData.rate. However, pool rate is also determined by last time a pool is updated. In _stake, _withdraw, and claim functions, whenever those functions gets executed, poolData.lastUpdate will be updated. When this happens right before another user tries to claim rewards, the reward calculation will also be affected:
function _getCurrentUserReward(uint256 currentPoolRate_, UserData memory userData_) private pure returns (uint256) {
uint256 newRewards_ = ((currentPoolRate_ - userData_.rate) * userData_.deposited) / PRECISION;
return userData_.pendingRewards + newRewards_;
}
function _getCurrentPoolRate(uint256 poolId_) private view returns (uint256) {
PoolData storage poolData = poolsData[poolId_];
if (poolData.totalDeposited == 0) {
return poolData.rate;
}
uint256 rewards_ = getPeriodReward(poolId_, poolData.lastUpdate, uint128(block.timestamp));
return poolData.rate + (rewards_ * PRECISION) / poolData.totalDeposited;
}
In the above code, we see rate is related to getPeriodReward, and in this functinon:
function getPeriodReward(
uint256 initialAmount_,
uint256 decreaseAmount_,
uint128 payoutStart_,
uint128 interval_,
uint128 startTime_,
uint128 endTime_
) external pure returns (uint256) {
if (interval_ == 0) {
return 0;
}
if (startTime_ < payoutStart_) {
startTime_ = payoutStart_;
}
uint128 maxEndTime_ = _calculateMaxEndTime(payoutStart_, interval_, initialAmount_, decreaseAmount_);
if (endTime_ > maxEndTime_) {
endTime_ = maxEndTime_;
}
if (startTime_ >= endTime_) {
return 0;
}
We see that when startTime >= endTime, the reward is 0. When this happens, the currentPoolRate variable will be reduced to the least possible value, and when the user tries to claim rewards, the value will be less than expected.
Impact
As mentioned above, reward claimers will get less reward than expected. In a way, this can be considered as a feature, but the cost of griefing is too low. Imagine this, suppose the minimum stake value is 1 stETH (10 ** 18), and griefer staked exactly the value. Now, the griefer sees another user trying to claim rewards, the griefer stakes again, but with only 1 wei of seETH. Since the requirement of minimum staking value has already been satisfied with the previous stake, the current one will also go through. Claimer will get less rewards, and the lost reward will actually go to griefer at a cost of only 1 wei. The griefer can always withdraw all staked stETH whenever he/she gets tried of all of these griefing, making the cost almost none.
When modify the following part of Distribution.t.ts, we can see how much claim will lost:
it('should correctly calculate and withdraw rewards', async () => {
let userData;
const wallet = ethers.Wallet.createRandom();
const randomAddress = wallet.address;
await setNextTime(oneHour * 2);
await distribution.manageUsersInPrivatePool(poolId, [secondAddress, ownerAddress, randomAddress], [wei(1), wei(4), wei(1)]);
await setNextTime(oneDay + oneDay * 1);
await distribution.claim(poolId, randomAddress, { value: wei(0.5) });
await distribution.claim(poolId, SECOND, { value: wei(0.5) });
await distribution.claim(poolId, OWNER, { value: wei(0.5) });
expect(await depositToken.balanceOf(secondAddress)).to.eq(wei(1000));
expect(await rewardToken.balanceOf(secondAddress)).to.eq(wei(20));
userData = await distribution.usersData(secondAddress, poolId);
expect(userData.deposited).to.eq(wei(1));
expect(userData.pendingRewards).to.eq(0);
expect(await depositToken.balanceOf(ownerAddress)).to.eq(wei(1000));
expect(await rewardToken.balanceOf(ownerAddress)).to.closeTo(wei(80), wei(0.001));
userData = await distribution.usersData(ownerAddress, poolId);
expect(userData.deposited).to.eq(wei(4));
expect(userData.pendingRewards).to.eq(0);
await setNextTime(oneDay + oneDay * 2);
await distribution.manageUsersInPrivatePool(poolId, [secondAddress, ownerAddress], [wei(0), wei(0)]);
expect(await depositToken.balanceOf(secondAddress)).to.eq(wei(1000));
expect(await rewardToken.balanceOf(secondAddress)).to.closeTo(wei(20), wei(0.001));
userData = await distribution.usersData(secondAddress, poolId);
expect(userData.deposited).to.eq(wei(0));
expect(userData.pendingRewards).to.closeTo(wei(19.6), wei(0.001));
expect(await depositToken.balanceOf(ownerAddress)).to.eq(wei(1000));
expect(await rewardToken.balanceOf(ownerAddress)).to.closeTo(wei(80), wei(0.001));
userData = await distribution.usersData(ownerAddress, poolId);
expect(userData.deposited).to.eq(wei(0));
expect(userData.pendingRewards).to.closeTo(wei(78.4), wei(0.001));
await distribution.claim(poolId, SECOND, { value: wei(0.5) });
});
This modifed test simply added an new address to the private pool, and frontruns secondAddress with a claim call. The result after running this test:
1) Distribution
#changeWhitelistedUsers
should correctly calculate and withdraw rewards:
AssertionError: expected 16666855709876543209 to equal 20000000000000000000. The numerical values of the given "bigint" and "bigint" inputs were compared, and they differed.
+ expected - actual
-16666855709876543209
+20000000000000000000
at Context.<anonymous> (test/Distribution.test.ts:423:61)
at
We see if not being frontran, the user is supposed to get 20000000000000000000 MOR tokens minted, but actually gets 16666855709876543209 This is around 16.7% less than expected. Of course, the loss can vary due to the last time pool is updated, but can still cause significant loss of rewards.
Tools Used
Manual review, hardhat.
Recommendations
If such action is supposed to be a feature, make the cost of griefing other user's reward higher, such as adding a minimum stake value for each single stake call.
