First Flight #9: Soulmate
First Flight #9
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: low
Valid

Staking Rewards Forfeited Upon Deposit Withdrawal

paprikrumplikas

Summary

Staking.sol is designed to reward users for locking their love tokens over a period. However, due to a flaw in the implementation, users who withdraw their deposited funds before claiming their accrued rewards forfeit these rewards. This behavior deviates from the expected outcome where rewards accrued during the staking period should be claimable until the moment of withdrawal.

Vulnerability Details

The vulnerability stems from the Staking contract's handling of reward claims and withdrawals. Specifically, the contract does not account for or preserve the rewards accrued by a user's stake when they perform a withdrawal. As demonstrated in the provided test code, after advancing time to allow for reward accrual and then withdrawing the initial deposit, the user's balance before and after attempting to claim rewards remains unchanged, indicating that the rewards were forfeited upon withdrawal.

function test_rewardsForfeitedUponDepositWithdrawal() public {
uint256 balanceUser1;
uint256 balanceUser1_beforeRewardsClaim;
uint256 balanceUser1_afterRewardsClaim;
uint256 totalAmountDeposited = 0;
​
// pair up user 1 and 2
vm.prank(user1);
soulmateContract.mintSoulmateToken();
vm.prank(user2);
soulmateContract.mintSoulmateToken();
​
vm.warp(block.timestamp + 4 weeks); // fast fwd time with 1 month
vm.startPrank(user1);
airdropContract.claim(); // claim airdrop
balanceUser1 = loveToken.balanceOf(user1);
totalAmountDeposited += balanceUser1;
loveToken.approve(address(stakingContract), type(uint256).max); // approve spending
stakingContract.deposit(balanceUser1); // deposit all
vm.stopPrank();
​
vm.warp(block.timestamp + 4 weeks); // fast fwd time with 1 month (due to another bug, not really neccesary)
​
vm.startPrank(user1);
stakingContract.withdraw(totalAmountDeposited);
balanceUser1_beforeRewardsClaim = loveToken.balanceOf(user1); // no rewards to claim
stakingContract.claimRewards();
balanceUser1_afterRewardsClaim = loveToken.balanceOf(user1);
vm.stopPrank();
​
assertEq(balanceUser1_beforeRewardsClaim, balanceUser1_afterRewardsClaim);
}

Impact

  1. Loss of Expected Rewards: Users lose potential rewards they have rightfully earned through staking, which can lead to dissatisfaction and reduced participation in the staking mechanism.

  2. Misalignment with Staking Incentives: The fundamental incentive for staking — earning rewards over time — is undermined if users risk losing rewards by withdrawing their stake.

Tools Used

Manual review.

Recommendations

Automate the rewards claim process during withdrawal to eliminate the need for users to manually claim rewards in a separate transaction:

function withdraw(uint256 amount) public {
+ claimRewards();
// No require needed because of overflow protection
userStakes[msg.sender] -= amount;
loveToken.transfer(msg.sender, amount);
emit Withdrew(msg.sender, amount);
}

Additionally, do clearly document and communicate this change to users, explaining how the automatic reward claim process works and its benefits, to maintain transparency and trust.

Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Other
paprikrumplikas Submitter
over 1 year ago
0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-withdraw-before-claimReward

If we we implement a correct claimRewards function with its intended logic, this would indeed be an issue. I believe low severity for this findings and its duplicates to be appropriate given it is dependent on users lack of understanding of claiming rewards first before a withdrawal.

Support

FAQs

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