MorpheusAI
MorpheusAI
Foundry
22,500 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

User needs unnecessary steps to get all their rewards.

ljj

Summary

User needs to call claim() then call withdraw() and then call claim() functions again to get all their rewards.

Vulnerability Details

A common user will leave the protocol by claiming their rewards by calling claim() function and then calling withdraw() function with all their funds (or vice versa). This however leaves the user with leftover rewards still in the system since _withdraw() function updates the users userData.pendingRewards

PoC

Implement a new line in claim() function to show amount of pending rewards when the test is ran.

function claim(uint256 poolId_, address user_) external payable poolExists(poolId_) {
Pool storage pool = pools[poolId_];
PoolData storage poolData = poolsData[poolId_];
UserData storage userData = usersData[user_][poolId_];
​
require(block.timestamp > pool.payoutStart + pool.claimLockPeriod, "DS: pool claim is locked");
uint256 currentPoolRate_ = _getCurrentPoolRate(poolId_);
uint256 pendingRewards_ = _getCurrentUserReward(currentPoolRate_, userData);
+++ console.log("pending rewards in claim:", pendingRewards_);
require(pendingRewards_ > 0, "DS: nothing to claim");

Implement the test shown below, under describe('#withdraw', () => { and run the test.

it.only('leaves left over rewards', async () => {
let userData;
​
await setNextTime(oneHour * 2);
await distribution.connect(SECOND).stake(poolId, wei(10));
await setNextTime(oneDay + oneDay);
await distribution.connect(OWNER).stake(poolId, wei(3));
await setNextTime(oneDay + oneDay * 2);
await distribution.claim(poolId, SECOND, { value: wei(0.5) });
await distribution.connect(SECOND).withdraw(poolId, wei(999));
await distribution.claim(poolId, SECOND, { value: wei(0.5) });

Console output is as shown below

Distribution
#withdraw
pending rewards: 0
pending rewards: 0
pending rewards in claim: 175384615384615384615
pending rewards in claim: 854700854700854

Test shows that after withdraw() is called user is left with rewards still in the system. If the user calls withdraw() first and then calls claim() they do not get the maximum amount of rewards they should.
So for a user to get maximum amount of rewards and have none left in the system they would need to call claim() first, then call withdraw() and then call claim() again.

Impact

User will not get the maximum amount of rewards they can.

Tools Used

Manual review.

Recommendations

_withdraw() function already checks if the user is withdrawing all their funds. Implement a new line in this function that will call claim() when a user is withdrawing all their funds.

Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
ljj Submitter
over 1 year ago
inallhonesty Lead Judge
over 1 year ago
inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.